ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jonathanhur...@apache.org
Subject ambari git commit: AMBARI-13915 - Storm Upgrade Causes NPE Due To null Property Value Replacement (jonathanhurley)
Date Tue, 17 Nov 2015 00:19:24 GMT
Repository: ambari
Updated Branches:
  refs/heads/trunk 2d82360b1 -> 004e4f98e


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/trunk
Commit: 004e4f98e7d20dbecdd31a7b889f55bfd1008897
Parents: 2d82360
Author: Jonathan Hurley <jhurley@hortonworks.com>
Authored: Mon Nov 16 16:31:31 2015 -0500
Committer: Jonathan Hurley <jhurley@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();


Mime
View raw message