SLIDER-470 slider appears to support negative component counts: client side checks
Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/1deafee8 Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/1deafee8 Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/1deafee8 Branch: refs/heads/develop Commit: 1deafee8401922f7609ce6333d38b3e52a43f06f Parents: ca88889 Author: Steve Loughran <[email protected]> Authored: Thu Oct 2 13:25:08 2014 -0700 Committer: Steve Loughran <[email protected]> Committed: Thu Oct 2 13:25:08 2014 -0700 ---------------------------------------------------------------------- .../org/apache/slider/client/SliderClient.java | 38 ++++++++++++++------ .../apache/slider/core/conf/MapOperations.java | 16 +++++++-- .../providers/AbstractClientProvider.java | 9 ++--- .../apache/slider/providers/ProviderCore.java | 5 +++ .../slideram/SliderAMClientProvider.java | 28 +++++++++++++++ .../slider/server/appmaster/state/AppState.java | 5 ++- .../server/appmaster/state/RoleStatus.java | 20 +++++++++++ .../standalone/TestBuildStandaloneAM.groovy | 25 +++++++++---- .../build/TestBuildThawClusterM1W1.groovy | 3 +- 9 files changed, 120 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/client/SliderClient.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/client/SliderClient.java b/slider-core/src/main/java/org/apache/slider/client/SliderClient.java index 06c37ba..05ef532 100644 --- a/slider-core/src/main/java/org/apache/slider/client/SliderClient.java +++ b/slider-core/src/main/java/org/apache/slider/client/SliderClient.java @@ -842,17 +842,10 @@ public class SliderClient extends AbstractSliderLaunchedService implements RunSe // make any substitutions needed at this stage replaceTokens(appConf.getConfTree(), getUsername(), clustername); - // provider to validate what there is - try { - sliderAM.validateInstanceDefinition(builder.getInstanceDescription()); - provider.validateInstanceDefinition(builder.getInstanceDescription()); - } catch (SliderException e) { - //problem, reject it - log.info("Error {} validating application instance definition ", e.toString()); - log.debug("Error validating application instance definition ", e); - log.info(instanceDefinition.toString()); - throw e; - } + // providers to validate what there is + AggregateConf instanceDescription = builder.getInstanceDescription(); + validateInstanceDefinition(sliderAM, instanceDescription); + validateInstanceDefinition(provider, instanceDescription); try { persistInstanceDefinition(overwrite, appconfdir, builder); } catch (LockAcquireFailedException e) { @@ -2060,6 +2053,10 @@ public class SliderClient extends AbstractSliderLaunchedService implements RunSe AggregateConf instanceDefinition = loadInstanceDefinitionUnresolved( clustername, clusterDirectory); + SliderAMClientProvider sliderAM = new SliderAMClientProvider(getConfig()); + // provider to validate what there is + SliderAMClientProvider provider = sliderAM; + validateInstanceDefinition(provider, instanceDefinition); ConfTreeOperations resources = instanceDefinition.getResourceOperations(); @@ -2107,6 +2104,25 @@ public class SliderClient extends AbstractSliderLaunchedService implements RunSe return exitCode; } + /** + * Validate an instance definition against a provider. + * @param provider the provider performing the validation + * @param instanceDefinition the instance definition + * @throws SliderException if invalid. + */ + protected void validateInstanceDefinition(AbstractClientProvider provider, + AggregateConf instanceDefinition) throws SliderException { + try { + provider.validateInstanceDefinition(instanceDefinition); + } catch (SliderException e) { + //problem, reject it + log.info("Error {} validating application instance definition ", e); + log.debug("Error validating application instance definition ", e); + log.info(instanceDefinition.toString()); + throw e; + } + } + /** * Load the persistent cluster description http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/core/conf/MapOperations.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/core/conf/MapOperations.java b/slider-core/src/main/java/org/apache/slider/core/conf/MapOperations.java index 4b1b44f..bba3ee2 100644 --- a/slider-core/src/main/java/org/apache/slider/core/conf/MapOperations.java +++ b/slider-core/src/main/java/org/apache/slider/core/conf/MapOperations.java @@ -53,16 +53,26 @@ public class MapOperations implements Map<String, String> { /** * Create an instance - * @param name - * @param options + * @param name name + * @param options source of options */ public MapOperations(String name, Map<String, String> options) { - assert options != null : "null map"; + Preconditions.checkArgument(options != null, "null map"); this.options = options; this.name = name; } /** + * Create an instance from an iterative map entry + * @param entry entry to work with + */ + public MapOperations(Map.Entry<String, Map<String, String>> entry) { + Preconditions.checkArgument(entry != null, "null entry"); + this.name = entry.getKey(); + this.options = entry.getValue(); + } + + /** * Get an option value * * @param key key http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/providers/AbstractClientProvider.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/providers/AbstractClientProvider.java b/slider-core/src/main/java/org/apache/slider/providers/AbstractClientProvider.java index f8008a4..7c2c7f4 100644 --- a/slider-core/src/main/java/org/apache/slider/providers/AbstractClientProvider.java +++ b/slider-core/src/main/java/org/apache/slider/providers/AbstractClientProvider.java @@ -62,7 +62,9 @@ public abstract class AbstractClientProvider extends Configured { public abstract List<ProviderRole> getRoles(); /** - * Validate the instance definition. + * Verify that an instance definition is considered valid by the provider + * @param instanceDefinition instance definition + * @throws SliderException if the configuration is not valid */ public void validateInstanceDefinition(AggregateConf instanceDefinition) throws SliderException { @@ -202,9 +204,8 @@ public abstract class AbstractClientProvider extends Configured { AggregateConf instanceDefinition, Path clusterDirPath, Path generatedConfDirPath, - boolean secure) throws - SliderException, - IOException { + boolean secure) + throws SliderException, IOException { validateInstanceDefinition(instanceDefinition); } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/providers/ProviderCore.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/providers/ProviderCore.java b/slider-core/src/main/java/org/apache/slider/providers/ProviderCore.java index f1883fc..65d2138 100644 --- a/slider-core/src/main/java/org/apache/slider/providers/ProviderCore.java +++ b/slider-core/src/main/java/org/apache/slider/providers/ProviderCore.java @@ -31,6 +31,11 @@ public interface ProviderCore { Configuration getConf(); + /** + * Verify that an instance definition is considered valid by the provider + * @param instanceDefinition instance definition + * @throws SliderException if the configuration is not valid + */ void validateInstanceDefinition(AggregateConf instanceDefinition) throws SliderException; } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/providers/slideram/SliderAMClientProvider.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/providers/slideram/SliderAMClientProvider.java b/slider-core/src/main/java/org/apache/slider/providers/slideram/SliderAMClientProvider.java index dc84f02..748e5f1 100644 --- a/slider-core/src/main/java/org/apache/slider/providers/slideram/SliderAMClientProvider.java +++ b/slider-core/src/main/java/org/apache/slider/providers/slideram/SliderAMClientProvider.java @@ -37,6 +37,7 @@ import org.apache.slider.common.tools.SliderFileSystem; import org.apache.slider.common.tools.SliderUtils; import org.apache.slider.core.conf.AggregateConf; import org.apache.slider.core.conf.MapOperations; +import org.apache.slider.core.exceptions.BadClusterStateException; import org.apache.slider.core.exceptions.BadConfigException; import org.apache.slider.core.exceptions.SliderException; import org.apache.slider.core.launch.AbstractLauncher; @@ -55,6 +56,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.apache.slider.api.ResourceKeys.COMPONENT_INSTANCES; + /** * handles the setup of the Slider AM. * This keeps aspects of role, cluster validation and Clusterspec setup @@ -132,6 +135,31 @@ public class SliderAMClientProvider extends AbstractClientProvider } /** + * Verify that an instance definition is considered valid by the provider + * @param instanceDefinition instance definition + * @throws SliderException if the configuration is not valid + */ + public void validateInstanceDefinition(AggregateConf instanceDefinition) throws + SliderException { + + super.validateInstanceDefinition(instanceDefinition); + + // make sure there is no negative entry in the instance count + Map<String, Map<String, String>> instanceMap = + instanceDefinition.getResources().components; + for (Map.Entry<String, Map<String, String>> entry : instanceMap.entrySet()) { + MapOperations mapOperations = new MapOperations(entry); + int instances = mapOperations.getOptionInt(COMPONENT_INSTANCES, 0); + if (instances < 0) { + throw new BadClusterStateException( + "Component %s has invalid instance count: %d", + mapOperations.name, + instances); + } + } + } + + /** * The Slider AM sets up all the dependency JARs above slider.jar itself * {@inheritDoc} */ http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java index abb6fe8..02a69f5 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java @@ -640,9 +640,8 @@ public class AppState { * The resource configuration is updated -review and update state. * @param resources updated resources specification */ - public synchronized void updateResourceDefinitions(ConfTree resources) throws - BadConfigException, - IOException { + public synchronized void updateResourceDefinitions(ConfTree resources) + throws BadConfigException, IOException { log.debug("Updating resources to {}", resources); instanceDefinition.setResources(resources); http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleStatus.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleStatus.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleStatus.java index df4ab8e..a112799 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleStatus.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleStatus.java @@ -47,6 +47,23 @@ public final class RoleStatus implements Cloneable { private int desired, actual, requested, releasing; private volatile int failed, started, startFailed, completed, totalRequested; + /** + * value to use when specifiying "no limit" for instances: {@value} + */ + public static final int UNLIMITED_INSTANCES = 1; + + /** + * minimum number of instances of a role permitted in a valid + * configuration. Default: 0. + */ + private int minimum = 0; + + /** + * maximum number of instances of a role permitted in a valid + * configuration. Default: unlimited. + */ + private int maximum = UNLIMITED_INSTANCES; + private String failureMessage = ""; public RoleStatus(ProviderRole providerRole) { @@ -208,6 +225,7 @@ public final class RoleStatus implements Cloneable { return totalRequested; } + /** * Get the number of roles we are short of. * nodes released are ignored. @@ -233,6 +251,8 @@ public final class RoleStatus implements Cloneable { return "RoleStatus{" + "name='" + name + '\'' + ", key=" + key + + ", minimum=" + minimum + + ", maximum=" + maximum + ", desired=" + desired + ", actual=" + actual + ", requested=" + requested + http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestBuildStandaloneAM.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestBuildStandaloneAM.groovy b/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestBuildStandaloneAM.groovy index 2058caf..a14a14d 100644 --- a/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestBuildStandaloneAM.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestBuildStandaloneAM.groovy @@ -65,14 +65,13 @@ class TestBuildStandaloneAM extends AgentMiniClusterTestBase { //but the cluster is still there for the default assert 0 == sliderClient.actionExists(clustername, false) - - - + + // verify the YARN registry doesn't know of it def serviceRegistryClient = sliderClient.YARNRegistryClient ApplicationReport report = serviceRegistryClient.findInstance(clustername) assert report == null; - + // verify that global resource options propagate from the CLI def aggregateConf = sliderClient.loadPersistedClusterDescription(clustername) def windowDays = aggregateConf.resourceOperations.globalOptions.getMandatoryOptionInt( @@ -94,13 +93,27 @@ class TestBuildStandaloneAM extends AgentMiniClusterTestBase { assertExceptionDetails(e, SliderExitCodes.EXIT_INSTANCE_EXISTS, "") } - - //start time ServiceLauncher<SliderClient> l2 = thawCluster(clustername, [], true) SliderClient thawed = l2.service addToTeardown(thawed); waitForClusterLive(thawed) + + // in the same code (for speed), create an illegal cluster + try { + ServiceLauncher<SliderClient> cluster3 = createOrBuildCluster( + SliderActions.ACTION_BUILD, + "illegalcluster", + ["role1": -1], + [], + false, + false, + agentDefOptions) + fail("expected an exception, got $cluster3.service") + } catch (SliderException e) { + assertExceptionDetails(e, SliderExitCodes.EXIT_BAD_STATE, "role1") + } + } @Test http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/1deafee8/slider-providers/hbase/slider-hbase-provider/src/test/groovy/org/apache/slider/providers/hbase/minicluster/build/TestBuildThawClusterM1W1.groovy ---------------------------------------------------------------------- diff --git a/slider-providers/hbase/slider-hbase-provider/src/test/groovy/org/apache/slider/providers/hbase/minicluster/build/TestBuildThawClusterM1W1.groovy b/slider-providers/hbase/slider-hbase-provider/src/test/groovy/org/apache/slider/providers/hbase/minicluster/build/TestBuildThawClusterM1W1.groovy index e4acb6b..8057b71 100644 --- a/slider-providers/hbase/slider-hbase-provider/src/test/groovy/org/apache/slider/providers/hbase/minicluster/build/TestBuildThawClusterM1W1.groovy +++ b/slider-providers/hbase/slider-hbase-provider/src/test/groovy/org/apache/slider/providers/hbase/minicluster/build/TestBuildThawClusterM1W1.groovy @@ -28,7 +28,6 @@ import org.apache.hadoop.yarn.api.records.ApplicationReport import org.apache.slider.core.main.ServiceLauncher import org.junit.Test -import static HBaseKeys.PROVIDER_HBASE import static org.apache.slider.common.params.Arguments.ARG_PROVIDER @CompileStatic @@ -50,7 +49,7 @@ class TestBuildThawClusterM1W1 extends HBaseMiniClusterTestBase { (HBaseKeys.ROLE_WORKER): 1, ], [ - ARG_PROVIDER, PROVIDER_HBASE + ARG_PROVIDER, HBaseKeys.PROVIDER_HBASE ], true, false,
