You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by nc...@apache.org on 2015/11/18 17:51:03 UTC
[07/50] [abbrv] ambari git commit: AMBARI-13915 - Storm Upgrade
Causes NPE Due To null Property Value Replacement (jonathanhurley)
AMBARI-13915 - Storm Upgrade Causes NPE Due To null Property Value Replacement (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/004e4f98
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/004e4f98
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/004e4f98
Branch: refs/heads/branch-dev-patch-upgrade
Commit: 004e4f98e7d20dbecdd31a7b889f55bfd1008897
Parents: 2d82360
Author: Jonathan Hurley <jh...@hortonworks.com>
Authored: Mon Nov 16 16:31:31 2015 -0500
Committer: Jonathan Hurley <jh...@hortonworks.com>
Committed: Mon Nov 16 19:19:07 2015 -0500
----------------------------------------------------------------------
.../serveraction/upgrades/ConfigureAction.java | 27 +++++---
.../upgrades/ConfigureActionTest.java | 72 +++++++++++++++++++-
2 files changed, 86 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/004e4f98/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 ef21a2a..706f9c6 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
@@ -45,11 +45,11 @@ import org.apache.ambari.server.state.ConfigMergeHelper.ThreeWayValue;
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.ConfigUpgradeChangeDefinition.ConfigurationKeyValue;
-import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Transfer;
-import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Replace;
import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Masked;
+import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Replace;
+import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Transfer;
+import org.apache.ambari.server.state.stack.upgrade.ConfigureTask;
import org.apache.commons.lang.StringUtils;
import com.google.gson.Gson;
@@ -404,9 +404,10 @@ public class ConfigureAction extends AbstractServerAction {
// !!! string replacements happen only on the new values.
for (Replace replacement : replacements) {
- if (newValues.containsKey(replacement.key)) {
- String toReplace = newValues.get(replacement.key);
-
+ // the key might exist but might be null, so we need to check this
+ // condition when replacing a part of the value
+ String toReplace = newValues.get(replacement.key);
+ if (StringUtils.isNotBlank(toReplace)) {
if (!toReplace.contains(replacement.find)) {
outputBuffer.append(MessageFormat.format("String \"{0}\" was not found in {1}/{2}\n",
replacement.find, configType, replacement.key));
@@ -415,16 +416,20 @@ public class ConfigureAction extends AbstractServerAction {
newValues.put(replacement.key, replaced);
- outputBuffer.append(MessageFormat.format("Replaced {0}/{1} containing \"{2}\" with \"{3}\"\n",
- configType, replacement.key, replacement.find, replacement.replaceWith));
+ outputBuffer.append(
+ MessageFormat.format("Replaced {0}/{1} containing \"{2}\" with \"{3}\"", configType,
+ replacement.key, replacement.find, replacement.replaceWith));
+
+ outputBuffer.append(System.lineSeparator());
}
} else {
- outputBuffer.append(MessageFormat.format("Property \"{0}\" was not found in {1} to replace content\n",
- replacement.key, configType));
+ outputBuffer.append(MessageFormat.format(
+ "Skipping replacement for {0}/{1} because it does not exist or is empty.",
+ configType, replacement.key));
+ outputBuffer.append(System.lineSeparator());
}
}
-
// !!! check to see if we're going to a new stack and double check the
// configs are for the target. Then simply update the new properties instead
// of creating a whole new history record since it was already done
http://git-wip-us.apache.org/repos/asf/ambari/blob/004e4f98/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 d1d783c..d7a2ad9 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
@@ -55,8 +55,10 @@ import org.apache.ambari.server.state.RepositoryVersionState;
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.ConfigUpgradeChangeDefinition.ConfigurationKeyValue;
+import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Replace;
+import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.Transfer;
import org.apache.ambari.server.state.stack.upgrade.ConfigureTask;
-import org.apache.ambari.server.state.stack.upgrade.ConfigUpgradeChangeDefinition.*;
import org.apache.ambari.server.state.stack.upgrade.TransferCoercionType;
import org.apache.ambari.server.state.stack.upgrade.TransferOperation;
import org.junit.After;
@@ -73,7 +75,7 @@ import com.google.inject.persist.PersistService;
* Tests upgrade-related server side actions
*/
public class ConfigureActionTest {
-
+
private static final String HDP_2_2_0_0 = "2.2.0.0-2041";
private static final String HDP_2_2_0_1 = "2.2.0.1-2270";
private static final StackId HDP_211_STACK = new StackId("HDP-2.1.1");
@@ -514,6 +516,72 @@ public class ConfigureActionTest {
assertEquals("WxyAndZ", config.getProperties().get("key_with_no_match"));
}
+ /**
+ * Tests that replacing a {@code null} value works.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testValueReplacementWithMissingConfigurations() throws Exception {
+ makeUpgradeCluster();
+
+ Cluster c = m_injector.getInstance(Clusters.class).getCluster("c1");
+ assertEquals(1, c.getConfigsByType("zoo.cfg").size());
+
+ c.setDesiredStackVersion(HDP_220_STACK);
+ ConfigFactory cf = m_injector.getInstance(ConfigFactory.class);
+ Config config = cf.createNew(c, "zoo.cfg", new HashMap<String, String>() {
+ {
+ put("existing", "This exists!");
+ put("missing", null);
+ }
+ }, 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());
+
+ Map<String, String> commandParams = new HashMap<String, String>();
+ commandParams.put("upgrade_direction", "upgrade");
+ commandParams.put("version", HDP_2_2_0_1);
+ commandParams.put("clusterName", "c1");
+ commandParams.put(ConfigureTask.PARAMETER_CONFIG_TYPE, "zoo.cfg");
+
+ // Replacement task
+ List<Replace> replacements = new ArrayList<Replace>();
+ Replace replace = new Replace();
+ replace.key = "missing";
+ replace.find = "foo";
+ replace.replaceWith = "bar";
+ replacements.add(replace);
+
+ commandParams.put(ConfigureTask.PARAMETER_REPLACEMENTS, new Gson().toJson(replacements));
+
+ ExecutionCommand executionCommand = new ExecutionCommand();
+ executionCommand.setCommandParams(commandParams);
+ executionCommand.setClusterName("c1");
+ executionCommand.setRoleParams(new HashMap<String, String>());
+ executionCommand.getRoleParams().put(ServerAction.ACTION_USER_NAME, "username");
+
+ 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(3, c.getConfigsByType("zoo.cfg").size());
+
+ config = c.getDesiredConfigByType("zoo.cfg");
+ assertEquals(null, config.getProperties().get("missing"));
+ }
+
@Test
public void testMultipleKeyValuesPerTask() throws Exception {
makeUpgradeCluster();