Repository: ambari Updated Branches: refs/heads/trunk c9b6c7874 -> 15000e505
AMBARI-11245 - ConfigurationTask Should Support Multiple Configurations Per Task (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/15000e50 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/15000e50 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/15000e50 Branch: refs/heads/trunk Commit: 15000e5057d0c072fbd5d6b1e40fe9e8b0300f61 Parents: c9b6c78 Author: Jonathan Hurley <[email protected]> Authored: Tue May 19 13:47:41 2015 -0400 Committer: Jonathan Hurley <[email protected]> Committed: Wed May 20 00:46:20 2015 -0400 ---------------------------------------------------------------------- .../internal/UpgradeResourceProvider.java | 49 ++----- .../serveraction/upgrades/ConfigureAction.java | 94 ++++++++----- .../state/stack/upgrade/ConfigureTask.java | 141 ++++++++++++------- .../stacks/HDP/2.2/upgrades/upgrade-2.3.xml | 19 +-- .../internal/UpgradeResourceProviderTest.java | 2 +- .../upgrades/ConfigureActionTest.java | 82 ++++++++++- .../ambari/server/state/UpgradeHelperTest.java | 89 ++++++++++-- .../stacks/HDP/2.1.1/upgrades/upgrade_test.xml | 22 ++- .../HDP/2.1.1/upgrades/upgrade_test_checks.xml | 3 +- .../stacks/HDP/2.2.0/upgrades/upgrade_test.xml | 3 +- .../HDP/2.2.0/upgrades/upgrade_test_checks.xml | 3 +- 11 files changed, 346 insertions(+), 161 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java index 81a15ec..a644d78 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java @@ -98,7 +98,6 @@ import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.gson.Gson; import com.google.inject.Inject; import com.google.inject.Provider; @@ -191,9 +190,6 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider @Inject private static HostDAO s_hostDAO = null; - private static Gson s_gson = new Gson(); - - /** * Used to generated the correct tasks and stages during an upgrade. */ @@ -1017,42 +1013,27 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider } case CONFIGURE: { ConfigureTask ct = (ConfigureTask) task; - Map<String, String> configProperties = ct.getConfigurationProperties(cluster); - List<ConfigureTask.Transfer> transfers = ct.getTransfers(); - List<ConfigureTask.Replace> replacements = ct.getReplacements(); - - // if the properties are empty it means that the conditions in the - // task did not pass; - if (configProperties.isEmpty() && transfers.isEmpty() && replacements.isEmpty()) { - stageText = "No conditions were met for this configuration task."; - itemDetail = stageText; - } else { - commandParams.putAll(configProperties); - commandParams.put(ConfigureTask.PARAMETER_TRANSFERS, s_gson.toJson(transfers)); - commandParams.put(ConfigureTask.PARAMETER_REPLACEMENTS, s_gson.toJson(replacements)); + Map<String, String> configurationChanges = ct.getConfigurationChanges(cluster); - // extract the config type, key and value to use to build the - // summary and detail - String configType = configProperties.get(ConfigureTask.PARAMETER_CONFIG_TYPE); - String key = configProperties.get(ConfigureTask.PARAMETER_KEY); - String value = configProperties.get(ConfigureTask.PARAMETER_VALUE); + // add all configuration changes to the command params + commandParams.putAll(configurationChanges); - StringBuilder detail = new StringBuilder(String.format("Updating config %s", configType)); - - if (null != key && null != value) { - detail.append(String.format("/%s to %s", key, value)); - } + // extract the config type to build the summary + String configType = configurationChanges.get(ConfigureTask.PARAMETER_CONFIG_TYPE); + if (null != configType) { + itemDetail = String.format("Updating configuration %s", configType); + } else { + itemDetail = "Skipping Configuration Task"; + } - itemDetail = detail.toString(); + entity.setText(itemDetail); - if (null != ct.summary) { - stageText = ct.summary; - } else { - stageText = String.format("Updating Config %s", configType); - } + if (null != ct.summary) { + stageText = ct.summary; + } else { + stageText = itemDetail; } - entity.setText(itemDetail); break; } default: http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java index 03b9fa3..c5f33c4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/ConfigureAction.java @@ -46,6 +46,7 @@ import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.PropertyInfo; import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.stack.upgrade.ConfigureTask; +import org.apache.ambari.server.state.stack.upgrade.ConfigureTask.ConfigurationKeyValue; import org.apache.commons.lang.StringUtils; import com.google.gson.Gson; @@ -95,7 +96,11 @@ public class ConfigureAction extends AbstractServerAction { @Inject private ConfigMergeHelper m_mergeHelper; - private Gson m_gson = new Gson(); + /** + * Gson + */ + @Inject + private Gson m_gson; /** * Aside from the normal execution, this method performs the following logic, with @@ -151,12 +156,19 @@ public class ConfigureAction extends AbstractServerAction { } String clusterName = commandParameters.get("clusterName"); + // such as hdfs-site or hbase-env String configType = commandParameters.get(ConfigureTask.PARAMETER_CONFIG_TYPE); - String key = commandParameters.get(ConfigureTask.PARAMETER_KEY); - String value = commandParameters.get(ConfigureTask.PARAMETER_VALUE); + // extract transfers + List<ConfigureTask.ConfigurationKeyValue> keyValuePairs = Collections.emptyList(); + String keyValuePairJson = commandParameters.get(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS); + if (null != keyValuePairJson) { + keyValuePairs = m_gson.fromJson( + keyValuePairJson, new TypeToken<List<ConfigureTask.ConfigurationKeyValue>>(){}.getType()); + } + // extract transfers List<ConfigureTask.Transfer> transfers = Collections.emptyList(); String transferJson = commandParameters.get(ConfigureTask.PARAMETER_TRANSFERS); if (null != transferJson) { @@ -164,6 +176,7 @@ public class ConfigureAction extends AbstractServerAction { transferJson, new TypeToken<List<ConfigureTask.Transfer>>(){}.getType()); } + // extract replacements List<ConfigureTask.Replace> replacements = Collections.emptyList(); String replaceJson = commandParameters.get(ConfigureTask.PARAMETER_REPLACEMENTS); if (null != replaceJson) { @@ -171,19 +184,26 @@ public class ConfigureAction extends AbstractServerAction { replaceJson, new TypeToken<List<ConfigureTask.Replace>>(){}.getType()); } - // if the two required properties are null and no transfer properties, then - // assume that no conditions were met and let the action complete - if (null == configType && null == key && transfers.isEmpty() && replacements.isEmpty()) { - return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", "", - "Skipping configuration task"); + // if there is nothing to do, then skip the task + if (keyValuePairs.isEmpty() && transfers.isEmpty() && replacements.isEmpty()) { + String message = "cluster={0}, type={1}, transfers={2}, replacements={3}, configurations={4}"; + message = MessageFormat.format(message, clusterName, configType, transfers, replacements, + keyValuePairs); + + StringBuilder buffer = new StringBuilder( + "Skipping this configuration task since none of the conditions were met and there are no transfers or replacements").append("\n"); + + buffer.append(message); + + return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", buffer.toString(), ""); } // if only 1 of the required properties was null and no transfer properties, // then something went wrong - if (null == clusterName || null == configType || - (null == key && transfers.isEmpty() && replacements.isEmpty())) { - String message = "cluster={0}, type={1}, key={2}, transfers={3}, replacements={4}"; - message = MessageFormat.format(message, clusterName, configType, key, transfers, replacements); + if (null == clusterName || null == configType + || (keyValuePairs.isEmpty() && transfers.isEmpty() && replacements.isEmpty())) { + String message = "cluster={0}, type={1}, transfers={2}, replacements={3}, configurations={4}"; + message = MessageFormat.format(message, clusterName, configType, transfers, replacements, keyValuePairs); return createCommandReport(0, HostRoleStatus.FAILED, "{}", "", message); } @@ -322,30 +342,39 @@ public class ConfigureAction extends AbstractServerAction { } } + // set all key/value pairs + if (null != keyValuePairs && !keyValuePairs.isEmpty()) { + for (ConfigurationKeyValue keyValuePair : keyValuePairs) { + String key = keyValuePair.key; + String value = keyValuePair.value; - if (null != key) { - String oldValue = base.get(key); + if (null != key) { + String oldValue = base.get(key); - // !!! values are not changing, so make this a no-op - if (null != oldValue && value.equals(oldValue)) { - if (currentStack.equals(targetStack) && !changedValues) { - return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", - MessageFormat.format("{0}/{1} for cluster {2} would not change, skipping setting", - configType, key, clusterName), - ""); + // !!! values are not changing, so make this a no-op + if (null != oldValue && value.equals(oldValue)) { + if (currentStack.equals(targetStack) && !changedValues) { + outputBuffer.append(MessageFormat.format( + "{0}/{1} for cluster {2} would not change, skipping setting", configType, key, + clusterName)); + + // continue because this property is not changing + continue; + } + } + + // !!! only put a key/value into this map of new configurations if + // there was a key, otherwise this will put something like null=null + // into the configs which will cause NPEs after upgrade - this is a + // byproduct of the configure being able to take a list of transfers + // without a key/value to set + newValues.put(key, value); + outputBuffer.append(MessageFormat.format("{0}/{1} changed to {2}\n", configType, key, + value)); } } - - // !!! only put a key/value into this map of new configurations if there - // was a key, otherwise this will put something like null=null into the - // configs which will cause NPEs after upgrade - this is a byproduct of - // the configure being able to take a list of transfers without a - // key/value to set - newValues.put(key, value); - outputBuffer.append(MessageFormat.format("{0}/{1} changed to {2}\n", configType, key, value)); } - // !!! string replacements happen only on the new values. for (ConfigureTask.Replace replacement : replacements) { if (newValues.containsKey(replacement.key)) { @@ -389,9 +418,8 @@ public class ConfigureAction extends AbstractServerAction { m_configHelper.createConfigType(cluster, m_controller, configType, newValues, auditName, serviceVersionNote); - String message = "Updated configuration ''{0}'' with ''{1}={2}''"; - message = MessageFormat.format(message, configType, key, value); - + String message = "Finished updating configuration ''{0}''"; + message = MessageFormat.format(message, configType); return createCommandReport(0, HostRoleStatus.COMPLETED, "{}", message, ""); } http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java index 5b47ee6..99d6058 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ConfigureTask.java @@ -36,6 +36,8 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.DesiredConfig; +import com.google.gson.Gson; + /** * The {@link ConfigureTask} represents a configuration change. This task can be * defined with conditional statements that will only set values if a condition @@ -59,15 +61,16 @@ import org.apache.ambari.server.state.DesiredConfig; * } * </pre> * - * It's also possible to simple set a value directly without a precondition + * It's also possible to simple set values directly without a precondition * check. * * <pre> * {@code * <task xsi:type="configure"> * <type>hive-site</type> - * <key>hive.server2.thrift.port</key> - * <value>10010</value> + * <set key="hive.server2.thrift.port" value="10010"/> + * <set key="foo" value="bar"/> + * <set key="foobar" value="baz"/> * </task> * } * </pre> @@ -84,20 +87,14 @@ public class ConfigureTask extends ServerSideActionTask { public static final String PARAMETER_CONFIG_TYPE = "configure-task-config-type"; /** - * The key that represents the configuration key to change (ie - * hive.server2.thrift.port) - */ - public static final String PARAMETER_KEY = "configure-task-key"; - - /** - * The key that represents the configuration value to set on - * {@link #PARAMETER_CONFIG_TYPE}/{@link #PARAMETER_KEY}. + * Setting key/value pairs can be several per task, so they're passed in as a + * json-ified list of objects. */ - public static final String PARAMETER_VALUE = "configure-task-value"; + public static final String PARAMETER_KEY_VALUE_PAIRS = "configure-task-key-value-pairs"; /** - * Transfers can be several per task, so they're passed in as a json-ified list of - * objects. + * Transfers can be several per task, so they're passed in as a json-ified + * list of objects. */ public static final String PARAMETER_TRANSFERS = "configure-task-transfers"; @@ -108,6 +105,11 @@ public class ConfigureTask extends ServerSideActionTask { public static final String PARAMETER_REPLACEMENTS = "configure-task-replacements"; /** + * Gson + */ + private Gson m_gson = new Gson(); + + /** * Constructor. * */ @@ -121,11 +123,8 @@ public class ConfigureTask extends ServerSideActionTask { @XmlElement(name="type") private String configType; - @XmlElement(name="key") - private String key; - - @XmlElement(name="value") - private String value; + @XmlElement(name = "set") + private List<ConfigurationKeyValue> keyValuePairs; @XmlElement(name="summary") public String summary; @@ -139,6 +138,9 @@ public class ConfigureTask extends ServerSideActionTask { @XmlElement(name="replace") private List<Replace> replacements; + /** + * {@inheritDoc} + */ @Override public Type getType() { return type; @@ -151,6 +153,18 @@ public class ConfigureTask extends ServerSideActionTask { return configType; } + /** + * A key/value pair to set in the type specified by {@link ConfigureTask#type} + */ + @XmlAccessorType(XmlAccessType.FIELD) + @XmlType(name = "set") + public static class ConfigurationKeyValue { + @XmlAttribute(name = "key") + public String key; + + @XmlAttribute(name = "value") + public String value; + } /** * A conditional element that will only perform the configuration if the @@ -320,49 +334,78 @@ public class ConfigureTask extends ServerSideActionTask { * Gets a map containing the following properties pertaining to the * configuration value to change: * <ul> - * <li>type - the configuration type (ie hdfs-site)</li> - * <li>key - the lookup key for the type</li> - * <li>value - the value to set the key to</li> + * <li>{@link #PARAMETER_CONFIG_TYPE} - the configuration type (ie hdfs-site)</li> + * <li>{@link #PARAMETER_KEY_VALUE_PAIRS} - key/value pairs for the + * configurations</li> + * <li>{@link #PARAMETER_KEY_VALUE_PAIRS} - key/value pairs for the + * configurations</li> + * <li>{@link #PARAMETER_TRANSFERS} - COPY/MOVE/DELETE changes</li> + * <li>{@link #PARAMETER_REPLACEMENTS} - value replacements</li> * </ul> * * @param cluster * the cluster to use when retrieving conditional properties to test * against (not {@code null}). - * @return the a map containing the configuration type, key, and value to use - * when updating a configuration property (never {@code null). - * This could potentially be an empty map if no conditions are - * met. Callers should decide how to handle a configuration task - * that is unable to set any configuration values. + * @return the a map containing the changes to make. This could potentially be + * an empty map if no conditions are met. Callers should decide how to + * handle a configuration task that is unable to set any configuration + * values. */ - public Map<String, String> getConfigurationProperties(Cluster cluster) { + public Map<String, String> getConfigurationChanges(Cluster cluster) { Map<String, String> configParameters = new HashMap<String, String>(); - // if there are no conditions then just take the direct values - if (null == conditions || conditions.isEmpty()) { + // the first matched condition will win; conditions make configuration tasks singular in + // the properties that can be set - when there is a condition the task will only contain + // conditions + if( null != conditions && !conditions.isEmpty() ){ + for (Condition condition : conditions) { + String conditionConfigType = condition.conditionConfigType; + String conditionKey = condition.conditionKey; + String conditionValue = condition.conditionValue; + + // always add the condition's target type just so that we have one to + // return even if none of the conditions match + configParameters.put(PARAMETER_CONFIG_TYPE, condition.configType); + + // check the condition; if it passes, set the configuration properties + // and break + String checkValue = getDesiredConfigurationValue(cluster, + conditionConfigType, conditionKey); + + if (conditionValue.equals(checkValue)) { + List<ConfigurationKeyValue> configurations = new ArrayList<ConfigurationKeyValue>(1); + ConfigurationKeyValue keyValue = new ConfigurationKeyValue(); + keyValue.key = condition.key; + keyValue.value = condition.value; + configurations.add(keyValue); + + configParameters.put(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS, + m_gson.toJson(configurations)); + + return configParameters; + } + } + } + + // this task is not a condition task, so process the other elements normally + if (null != configType) { configParameters.put(PARAMETER_CONFIG_TYPE, configType); - configParameters.put(PARAMETER_KEY, key); - configParameters.put(PARAMETER_VALUE, value); - return configParameters; } - // the first matched condition will win; a single configuration task - // should only ever set a single configuration property - for (Condition condition : conditions) { - String conditionConfigType = condition.conditionConfigType; - String conditionKey = condition.conditionKey; - String conditionValue = condition.conditionValue; + // for every <set key=foo value=bar/> add it to this list + if (null != keyValuePairs && !keyValuePairs.isEmpty()) { + configParameters.put(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS, + m_gson.toJson(keyValuePairs)); + } - // check the condition; if it passes, set the configuration properties - // and break - String checkValue = getDesiredConfigurationValue(cluster, - conditionConfigType, conditionKey); + // transfers + if( null != transfers && !transfers.isEmpty() ){ + configParameters.put(ConfigureTask.PARAMETER_TRANSFERS, m_gson.toJson(transfers)); + } - if (conditionValue.equals(checkValue)) { - configParameters.put(PARAMETER_CONFIG_TYPE, condition.configType); - configParameters.put(PARAMETER_KEY, condition.key); - configParameters.put(PARAMETER_VALUE, condition.value); - break; - } + // replacements + if( null != replacements && !replacements.isEmpty() ){ + configParameters.put(ConfigureTask.PARAMETER_REPLACEMENTS, m_gson.toJson(replacements)); } return configParameters; http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml b/ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml index a620b05..37a75113 100644 --- a/ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml +++ b/ambari-server/src/main/resources/stacks/HDP/2.2/upgrades/upgrade-2.3.xml @@ -364,12 +364,7 @@ <type>mapred-site</type> <transfer operation="move" from-key="mapreduce.job.speculative.speculativecap" to-key="mapreduce.job.speculative.speculative-cap-running-tasks" default-value="0.1" /> <transfer operation="delete" delete-key="mapreduce.task.tmp.dir" /> - </task> - - <task xsi:type="configure"> - <type>mapred-site</type> - <key>mapreduce.fileoutputcommitter.algorithm.version</key> - <value>1</value> + <set key="mapreduce.fileoutputcommitter.algorithm.version" value="1"/> </task> </pre-upgrade> @@ -390,8 +385,7 @@ <pre-upgrade> <task xsi:type="configure"> <type>yarn-site</type> - <key>yarn.timeline-service.recovery.enabled</key> - <value>true</value> + <set key="yarn.timeline-service.recovery.enabled" value="true"/> </task> </pre-upgrade> @@ -404,8 +398,7 @@ <pre-upgrade> <task xsi:type="configure"> <type>yarn-site</type> - <key>yarn.node-labels.enabled</key> - <value>false</value> + <set key="yarn.node-labels.enabled" value="false"/> </task> <task xsi:type="configure"> <type>capacity-scheduler</type> @@ -456,13 +449,11 @@ <pre-upgrade> <task xsi:type="configure"> <type>tez-site</type> - <key>tez.am.view-acls</key> - <value>*</value> + <set key="tez.am.view-acls" value="*"/> </task> <task xsi:type="configure"> <type>tez-site</type> - <key>tez.task.generate.counters.per.io</key> - <value>true</value> + <set key="tez.task.generate.counters.per.io" value="true"/> </task> </pre-upgrade> <upgrade> http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java index 6ae51da..12e6efe 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java @@ -253,7 +253,7 @@ public class UpgradeResourceProviderTest { "placeholder of placeholder-rendered-properly")); assertTrue(group.getItems().get(1).getText().contains("Restarting")); - assertTrue(group.getItems().get(2).getText().contains("Updating")); + assertTrue(group.getItems().get(2).getText().contains("Skipping")); assertTrue(group.getItems().get(3).getText().contains("Service Check")); ActionManager am = injector.getInstance(ActionManager.class); http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java index 6ca677a..c2ea948 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/ConfigureActionTest.java @@ -56,6 +56,7 @@ import org.apache.ambari.server.state.Service; import org.apache.ambari.server.state.ServiceFactory; import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.stack.upgrade.ConfigureTask; +import org.apache.ambari.server.state.stack.upgrade.ConfigureTask.ConfigurationKeyValue; import org.apache.ambari.server.state.stack.upgrade.TransferCoercionType; import org.apache.ambari.server.state.stack.upgrade.TransferOperation; import org.junit.After; @@ -128,14 +129,18 @@ public class ConfigureActionTest { c.addDesiredConfig("user", Collections.singleton(config)); assertEquals(2, c.getConfigsByType("zoo.cfg").size()); + List<ConfigurationKeyValue> configurations = new ArrayList<ConfigureTask.ConfigurationKeyValue>(); + ConfigurationKeyValue keyValue = new ConfigurationKeyValue(); + configurations.add(keyValue); + keyValue.key = "initLimit"; + keyValue.value = "11"; Map<String, String> commandParams = new HashMap<String, String>(); commandParams.put("upgrade_direction", "upgrade"); commandParams.put("version", HDP_2_2_1_0); commandParams.put("clusterName", "c1"); commandParams.put(ConfigureTask.PARAMETER_CONFIG_TYPE, "zoo.cfg"); - commandParams.put(ConfigureTask.PARAMETER_KEY, "initLimit"); - commandParams.put(ConfigureTask.PARAMETER_VALUE, "11"); + commandParams.put(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS, new Gson().toJson(configurations)); ExecutionCommand executionCommand = new ExecutionCommand(); executionCommand.setCommandParams(commandParams); @@ -259,14 +264,18 @@ public class ConfigureActionTest { c.addDesiredConfig("user", Collections.singleton(config)); assertEquals(2, c.getConfigsByType("zoo.cfg").size()); + List<ConfigurationKeyValue> configurations = new ArrayList<ConfigureTask.ConfigurationKeyValue>(); + ConfigurationKeyValue keyValue = new ConfigurationKeyValue(); + configurations.add(keyValue); + keyValue.key = "initLimit"; + keyValue.value = "11"; Map<String, String> commandParams = new HashMap<String, String>(); commandParams.put("upgrade_direction", "upgrade"); commandParams.put("version", HDP_2_2_1_0); commandParams.put("clusterName", "c1"); commandParams.put(ConfigureTask.PARAMETER_CONFIG_TYPE, "zoo.cfg"); - commandParams.put(ConfigureTask.PARAMETER_KEY, "initLimit"); - commandParams.put(ConfigureTask.PARAMETER_VALUE, "11"); + commandParams.put(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS, new Gson().toJson(configurations)); // normal copy List<ConfigureTask.Transfer> transfers = new ArrayList<ConfigureTask.Transfer>(); @@ -431,8 +440,6 @@ public class ConfigureActionTest { assertEquals("['c6401','c6402','c6403']", map.get("zoo.server.array")); } - - @Test public void testValueReplacement() throws Exception { makeUpgradeCluster(); @@ -502,10 +509,71 @@ public class ConfigureActionTest { assertEquals("My Wet Dog", config.getProperties().get("key_to_replace")); assertEquals("WxyAndZ", config.getProperties().get("key_with_no_match")); - } + @Test + public void testMultipleKeyValuesPerTask() throws Exception { + makeUpgradeCluster(); + Cluster c = m_injector.getInstance(Clusters.class).getCluster("c1"); + assertEquals(1, c.getConfigsByType("zoo.cfg").size()); + + c.setDesiredStackVersion(HDP_22_STACK); + ConfigFactory cf = m_injector.getInstance(ConfigFactory.class); + Config config = cf.createNew(c, "zoo.cfg", new HashMap<String, String>() { + { + put("fooKey", "barValue"); + } + }, new HashMap<String, Map<String, String>>()); + + config.setTag("version2"); + config.persist(); + + c.addConfig(config); + c.addDesiredConfig("user", Collections.singleton(config)); + assertEquals(2, c.getConfigsByType("zoo.cfg").size()); + + // create several configurations + List<ConfigurationKeyValue> configurations = new ArrayList<ConfigureTask.ConfigurationKeyValue>(); + ConfigurationKeyValue fooKey2 = new ConfigurationKeyValue(); + configurations.add(fooKey2); + fooKey2.key = "fooKey2"; + fooKey2.value = "barValue2"; + + ConfigurationKeyValue fooKey3 = new ConfigurationKeyValue(); + configurations.add(fooKey3); + fooKey3.key = "fooKey3"; + fooKey3.value = "barValue3"; + + Map<String, String> commandParams = new HashMap<String, String>(); + commandParams.put("upgrade_direction", "upgrade"); + commandParams.put("version", HDP_2_2_1_0); + commandParams.put("clusterName", "c1"); + commandParams.put(ConfigureTask.PARAMETER_CONFIG_TYPE, "zoo.cfg"); + commandParams.put(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS, new Gson().toJson(configurations)); + + ExecutionCommand executionCommand = new ExecutionCommand(); + executionCommand.setCommandParams(commandParams); + executionCommand.setClusterName("c1"); + + HostRoleCommand hostRoleCommand = hostRoleCommandFactory.create(null, null, null, null); + hostRoleCommand.setExecutionCommandWrapper(new ExecutionCommandWrapper(executionCommand)); + + ConfigureAction action = m_injector.getInstance(ConfigureAction.class); + action.setExecutionCommand(executionCommand); + action.setHostRoleCommand(hostRoleCommand); + + CommandReport report = action.execute(null); + assertNotNull(report); + + assertEquals(2, c.getConfigsByType("zoo.cfg").size()); + + config = c.getDesiredConfigByType("zoo.cfg"); + assertNotNull(config); + assertEquals("barValue", config.getProperties().get("fooKey")); + assertEquals("barValue2", config.getProperties().get("fooKey2")); + assertEquals("barValue3", config.getProperties().get("fooKey3")); + } private void makeUpgradeCluster() throws Exception { String clusterName = "c1"; http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java index c51cf2d..f4e847c 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java @@ -44,7 +44,6 @@ import org.apache.ambari.server.controller.ConfigurationRequest; import org.apache.ambari.server.orm.GuiceJpaInitializer; import org.apache.ambari.server.orm.InMemoryDefaultTestModule; import org.apache.ambari.server.orm.OrmTestHelper; -import org.apache.ambari.server.orm.dao.HostDAO; import org.apache.ambari.server.stack.HostsType; import org.apache.ambari.server.stack.MasterHostResolver; import org.apache.ambari.server.state.UpgradeHelper.UpgradeGroupHolder; @@ -60,6 +59,8 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; import com.google.inject.Binder; import com.google.inject.Guice; import com.google.inject.Injector; @@ -84,7 +85,7 @@ public class UpgradeHelperTest { private UpgradeHelper m_upgradeHelper; private ConfigHelper m_configHelper; private AmbariManagementController m_managementController; - private HostDAO m_hostDAO; + private Gson m_gson = new Gson(); @Before public void before() throws Exception { @@ -110,7 +111,6 @@ public class UpgradeHelperTest { m_upgradeHelper = injector.getInstance(UpgradeHelper.class); m_masterHostResolver = EasyMock.createMock(MasterHostResolver.class); m_managementController = injector.getInstance(AmbariManagementController.class); - m_hostDAO = injector.getInstance(HostDAO.class); } @After @@ -394,11 +394,19 @@ public class UpgradeHelperTest { ConfigureTask configureTask = (ConfigureTask) hiveGroup.items.get(1).getTasks().get( 0).getTasks().get(0); - Map<String, String> configProperties = configureTask.getConfigurationProperties(cluster); + Map<String, String> configProperties = configureTask.getConfigurationChanges(cluster); assertFalse(configProperties.isEmpty()); - assertEquals( configProperties.get(ConfigureTask.PARAMETER_CONFIG_TYPE), "hive-site"); - assertEquals( configProperties.get(ConfigureTask.PARAMETER_KEY), "hive.server2.thrift.port"); - assertEquals( configProperties.get(ConfigureTask.PARAMETER_VALUE), "10010"); + assertEquals(configProperties.get(ConfigureTask.PARAMETER_CONFIG_TYPE), "hive-site"); + + String configurationJson = configProperties.get(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS); + assertNotNull(configurationJson); + + List<ConfigureTask.ConfigurationKeyValue> keyValuePairs = m_gson.fromJson(configurationJson, + new TypeToken<List<ConfigureTask.ConfigurationKeyValue>>() { + }.getType()); + + assertEquals("hive.server2.thrift.port", keyValuePairs.get(0).key); + assertEquals("10010", keyValuePairs.get(0).value); // now change the thrift port to http to have the 2nd condition invoked Map<String, String> hiveConfigs = new HashMap<String, String>(); @@ -422,11 +430,72 @@ public class UpgradeHelperTest { }, null); // the configure task should now return different properties - configProperties = configureTask.getConfigurationProperties(cluster); + configProperties = configureTask.getConfigurationChanges(cluster); assertFalse(configProperties.isEmpty()); assertEquals( configProperties.get(ConfigureTask.PARAMETER_CONFIG_TYPE), "hive-site"); - assertEquals( configProperties.get(ConfigureTask.PARAMETER_KEY), "hive.server2.http.port"); - assertEquals( configProperties.get(ConfigureTask.PARAMETER_VALUE), "10011"); + + configurationJson = configProperties.get(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS); + assertNotNull(configurationJson); + + keyValuePairs = m_gson.fromJson(configurationJson, + new TypeToken<List<ConfigureTask.ConfigurationKeyValue>>() { + }.getType()); + + assertEquals("hive.server2.http.port", keyValuePairs.get(0).key); + assertEquals("10011", keyValuePairs.get(0).value); + } + + @Test + public void testConfigureTaskWithMultipleConfigurations() throws Exception { + Map<String, UpgradePack> upgrades = ambariMetaInfo.getUpgradePacks("HDP", "2.1.1"); + + assertTrue(upgrades.containsKey("upgrade_test")); + UpgradePack upgrade = upgrades.get("upgrade_test"); + assertNotNull(upgrade); + + Cluster cluster = makeCluster(); + + UpgradeContext context = new UpgradeContext(m_masterHostResolver, HDP_21, HDP_21, + UPGRADE_VERSION, Direction.UPGRADE); + + List<UpgradeGroupHolder> groups = m_upgradeHelper.createSequence(upgrade, context); + + assertEquals(6, groups.size()); + + // grab the configure task out of Hive + UpgradeGroupHolder hiveGroup = groups.get(4); + assertEquals("HIVE", hiveGroup.name); + ConfigureTask configureTask = (ConfigureTask) hiveGroup.items.get(1).getTasks().get(1).getTasks().get(0); + + Map<String, String> configProperties = configureTask.getConfigurationChanges(cluster); + assertFalse(configProperties.isEmpty()); + assertEquals(configProperties.get(ConfigureTask.PARAMETER_CONFIG_TYPE), "hive-site"); + + String configurationJson = configProperties.get(ConfigureTask.PARAMETER_KEY_VALUE_PAIRS); + String transferJson = configProperties.get(ConfigureTask.PARAMETER_TRANSFERS); + assertNotNull(configurationJson); + assertNotNull(transferJson); + + List<ConfigureTask.ConfigurationKeyValue> keyValuePairs = m_gson.fromJson(configurationJson, + new TypeToken<List<ConfigureTask.ConfigurationKeyValue>>() { + }.getType()); + + List<ConfigureTask.Transfer> transfers = m_gson.fromJson(transferJson, + new TypeToken<List<ConfigureTask.Transfer>>() { + }.getType()); + + assertEquals("fooKey", keyValuePairs.get(0).key); + assertEquals("fooValue", keyValuePairs.get(0).value); + assertEquals("fooKey2", keyValuePairs.get(1).key); + assertEquals("fooValue2", keyValuePairs.get(1).value); + assertEquals("fooKey3", keyValuePairs.get(2).key); + assertEquals("fooValue3", keyValuePairs.get(2).value); + + assertEquals("copy-key", transfers.get(0).fromKey); + assertEquals("copy-key-to", transfers.get(0).toKey); + + assertEquals("move-key", transfers.get(1).fromKey); + assertEquals("move-key-to", transfers.get(1).toKey); } http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml b/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml index 0e5e53c..d168234 100644 --- a/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml +++ b/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test.xml @@ -141,8 +141,7 @@ </task> <task xsi:type="configure"> <type>hdfs-site</type> - <key>myproperty</key> - <value>mynewvalue</value> + <set key="myproperty" value="mynewvalue"/> </task> <task xsi:type="manual"> <message>{{direction.verb.proper}} your database</message> @@ -185,11 +184,11 @@ </task> <task xsi:type="configure"> <type>core-site</type> - <transfer operation="copy" from-key="copy-key" to-key="copy-key-to" /> - <transfer operation="copy" from-type="my-site" from-key="my-copy-key" to-key="my-copy-key-to" /> - <transfer operation="move" from-key="move-key" to-key="move-key-to" /> - <transfer operation="delete" delete-key="delete-key" preserve-edits="true"> - <keep-key>important-key</keep-key> + <transfer operation="copy" from-key="copy-key" to-key="copy-key-to" /> + <transfer operation="copy" from-type="my-site" from-key="my-copy-key" to-key="my-copy-key-to" /> + <transfer operation="move" from-key="move-key" to-key="move-key-to" /> + <transfer operation="delete" delete-key="delete-key" preserve-edits="true"> + <keep-key>important-key</keep-key> </transfer> </task> </pre-upgrade> @@ -216,6 +215,15 @@ <value>10011</value> </condition> </task> + + <task xsi:type="configure"> + <type>hive-site</type> + <set key="fooKey" value="fooValue"/> + <set key="fooKey2" value="fooValue2"/> + <set key="fooKey3" value="fooValue3"/> + <transfer operation="copy" from-key="copy-key" to-key="copy-key-to" /> + <transfer operation="move" from-key="move-key" to-key="move-key-to" /> + </task> </pre-upgrade> </component> </service> http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml b/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml index bbaa178..b4b6663 100644 --- a/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml +++ b/ambari-server/src/test/resources/stacks/HDP/2.1.1/upgrades/upgrade_test_checks.xml @@ -134,8 +134,7 @@ </task> <task xsi:type="configure"> <type>hdfs-site</type> - <key>myproperty</key> - <value>mynewvalue</value> + <set key="myproperty" value="mynewvalue"/> </task> <task xsi:type="manual"> <message>Update your database</message> http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test.xml ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test.xml b/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test.xml index 08e69d7..f321d4c 100644 --- a/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test.xml +++ b/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test.xml @@ -141,8 +141,7 @@ </task> <task xsi:type="configure"> <type>hdfs-site</type> - <key>myproperty</key> - <value>mynewvalue</value> + <set key="myproperty" value="mynewvalue"/> </task> <task xsi:type="manual"> <message>{{direction.verb.proper}} your database</message> http://git-wip-us.apache.org/repos/asf/ambari/blob/15000e50/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_checks.xml ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_checks.xml b/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_checks.xml index 4ad019c..892b9b4 100644 --- a/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_checks.xml +++ b/ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_test_checks.xml @@ -140,8 +140,7 @@ </task> <task xsi:type="configure"> <type>hdfs-site</type> - <key>myproperty</key> - <value>mynewvalue</value> + <set key="myproperty" value="mynewvalue"/> </task> <task xsi:type="manual"> <message>Update your database</message>
