Repository: ambari
Updated Branches:
  refs/heads/trunk aedb50c0e -> 59f15b2ac


AMBARI-11177 - ConfigureTask For Upgrades Must Support Type Coercion 
(jonathanhurley)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/59f15b2a
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/59f15b2a
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/59f15b2a

Branch: refs/heads/trunk
Commit: 59f15b2ac55d6e92212a8c9a7afae7565633df46
Parents: aedb50c
Author: Jonathan Hurley <jhur...@hortonworks.com>
Authored: Fri May 15 12:49:48 2015 -0400
Committer: Jonathan Hurley <jhur...@hortonworks.com>
Committed: Fri May 15 14:56:25 2015 -0400

----------------------------------------------------------------------
 .../serveraction/upgrades/ConfigureAction.java  | 78 ++++++++++----------
 .../state/stack/upgrade/ConfigureTask.java      | 34 ++++++++-
 .../stack/upgrade/TransferCoercionType.java     | 34 +++++++++
 .../state/stack/upgrade/TransferOperation.java  |  7 ++
 .../stacks/HDP/2.2/upgrades/upgrade-2.3.xml     | 27 +++----
 .../upgrades/ConfigureActionTest.java           | 68 +++++++++++++++++
 6 files changed, 197 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/59f15b2a/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 645c4bb..729c36a 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
@@ -200,50 +200,54 @@ public class ConfigureAction extends AbstractServerAction 
{
     for (ConfigureTask.Transfer transfer : transfers) {
       switch (transfer.operation) {
         case COPY:
-          // if copying from the current configuration type, then first
-          // determine if the key already exists; if it does not, then set a
-          // default if a default exists
-          if (null == transfer.fromType) {
-            if (base.containsKey(transfer.fromKey)) {
-              newValues.put(transfer.toKey, base.get(transfer.fromKey));
-              changedValues = true;
-
-              // append standard output
-              outputBuffer.append(MessageFormat.format("Copied {0}/{1}\n", 
configType,
-                  transfer.toKey));
-
-            } else if (StringUtils.isNotBlank(transfer.defaultValue)) {
-              newValues.put(transfer.toKey, transfer.defaultValue);
-              changedValues = true;
-
-              // append standard output
-              outputBuffer.append(MessageFormat.format("Created {0}/{1} with 
default value {2}\n",
-                  configType, transfer.toKey, transfer.defaultValue));
-            }
+          String valueToCopy = null;
+          if( null == transfer.fromType ) {
+            // copying from current configuration
+            valueToCopy = base.get(transfer.fromKey);
           } else {
-            // !!! copying from another configuration
+            // copying from another configuration
             Config other = cluster.getDesiredConfigByType(transfer.fromType);
-
-            if (null != other) {
+            if (null != other){
               Map<String, String> otherValues = other.getProperties();
+              if (otherValues.containsKey(transfer.fromKey)){
+                valueToCopy = otherValues.get(transfer.fromKey);
+              }
+            }
+          }
 
-              if (otherValues.containsKey(transfer.fromKey)) {
-                newValues.put(transfer.toKey, 
otherValues.get(transfer.fromKey));
-                changedValues = true;
-
-                // append standard output
-                outputBuffer.append(MessageFormat.format("Copied {0}/{1} to 
{2}\n",
-                    transfer.fromType, transfer.fromKey, configType));
-              } else if (StringUtils.isNotBlank(transfer.defaultValue)) {
-                newValues.put(transfer.toKey, transfer.defaultValue);
-                changedValues = true;
+          // if the value is null use the default if it exists
+          if (StringUtils.isBlank(valueToCopy) && 
!StringUtils.isBlank(transfer.defaultValue)) {
+            valueToCopy = transfer.defaultValue;
+          }
 
-                // append standard output
-                outputBuffer.append(MessageFormat.format(
-                    "Created {0}/{1} with default value {2}\n", configType, 
transfer.toKey,
-                    transfer.defaultValue));
+          if (StringUtils.isNotBlank(valueToCopy)) {
+            // possibly coerce the value on copy
+            if (transfer.coerceTo != null) {
+              switch (transfer.coerceTo) {
+                case YAML_ARRAY: {
+                  // turn c6401,c6402 into ['c6401',c6402']
+                  String[] splitValues = StringUtils.split(valueToCopy, ',');
+                  List<String> quotedValues = new 
ArrayList<String>(splitValues.length);
+                  for (String splitValue : splitValues) {
+                    quotedValues.add("'" + StringUtils.trim(splitValue) + "'");
+                  }
+
+                  valueToCopy = "[" + StringUtils.join(quotedValues, ',') + 
"]";
+
+                  break;
+                }
+                default:
+                  break;
               }
             }
+
+            // at this point we know that we have a changed value
+            changedValues = true;
+            newValues.put(transfer.toKey, valueToCopy);
+
+            // append standard output
+            outputBuffer.append(MessageFormat.format("Created {0}/{1} = 
{2}\n", configType,
+                transfer.toKey, valueToCopy));
           }
           break;
         case MOVE:

http://git-wip-us.apache.org/repos/asf/ambari/blob/59f15b2a/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 2b68a28..06d3108 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
@@ -175,30 +175,62 @@ public class ConfigureTask extends ServerSideActionTask {
   @XmlAccessorType(XmlAccessType.FIELD)
   @XmlType(name = "transfer")
   public static class Transfer {
+    /**
+     * The type of operation, such as COPY or DELETE.
+     */
     @XmlAttribute(name = "operation")
     public TransferOperation operation;
 
+    /**
+     * The configuration type to copy or move from.
+     */
     @XmlAttribute(name = "from-type")
     public String fromType;
 
+    /**
+     * The key to copy or move the configuration from.
+     */
     @XmlAttribute(name = "from-key")
     public String fromKey;
 
+    /**
+     * The key to copy the configuration value to.
+     */
     @XmlAttribute(name = "to-key")
     public String toKey;
 
+    /**
+     * The configuration key to delete, or "*" for all.
+     */
     @XmlAttribute(name = "delete-key")
     public String deleteKey;
 
+    /**
+     * If {@code true}, this will ensure that any changed properties are not
+     * removed during a {@link TransferOperation#DELETE}.
+     */
     @XmlAttribute(name = "preserve-edits")
     public boolean preserveEdits = false;
 
+    /**
+     * A default value to use when the configurations don't contain the
+     * {@link #fromKey}.
+     */
     @XmlAttribute(name = "default-value")
     public String defaultValue;
 
+    /**
+     * A data type to convert the configuration value to when the action is
+     * {@link TransferOperation#COPY}.
+     */
+    @XmlAttribute(name = "coerce-to")
+    public TransferCoercionType coerceTo;
+
+    /**
+     * The keys to keep when the action is {@link TransferOperation#DELETE}.
+     */
     @XmlElement(name = "keep-key")
     public List<String> keepKeys = new ArrayList<String>();
-
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/ambari/blob/59f15b2a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferCoercionType.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferCoercionType.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferCoercionType.java
new file mode 100644
index 0000000..03fd249
--- /dev/null
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferCoercionType.java
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.state.stack.upgrade;
+
+import javax.xml.bind.annotation.XmlEnum;
+import javax.xml.bind.annotation.XmlEnumValue;
+
+/**
+ * The {@link TransferCoercionType} enum is used to represent the data types
+ * that a property transfer can be coerced to.
+ */
+@XmlEnum
+public enum TransferCoercionType {
+  /**
+   * Convert to a YAML array.
+   */
+  @XmlEnumValue("yaml-array")
+  YAML_ARRAY
+}

http://git-wip-us.apache.org/repos/asf/ambari/blob/59f15b2a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferOperation.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferOperation.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferOperation.java
index 1ee2a35..160de71 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferOperation.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TransferOperation.java
@@ -17,24 +17,31 @@
  */
 package org.apache.ambari.server.state.stack.upgrade;
 
+import javax.xml.bind.annotation.XmlEnum;
+import javax.xml.bind.annotation.XmlEnumValue;
+
 /**
  * Operations valid for a property transfer.
  */
+@XmlEnum
 public enum TransferOperation {
   /**
    * The property should be removed.  Special case "*" to delete all
    * properties that have NOT been overridden by a user.
    */
+  @XmlEnumValue("delete")
   DELETE,
   /**
    * The property value is moved.  A property may only be moved within the
    * same config type.  To move across types, perform a {@link #COPY} to the 
target then
    * a {@link #DELETE} from the source config.
    */
+  @XmlEnumValue("move")
   MOVE,
   /**
    * The property value is copied from another property.  A property may be 
copied from
    * another config type.
    */
+  @XmlEnumValue("copy")
   COPY
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/59f15b2a/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 eb89dbd..43db5b0 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
@@ -362,8 +362,8 @@
         <pre-upgrade>
           <task xsi:type="configure">
             <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" />
+            <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">
@@ -396,7 +396,7 @@
 
           <task xsi:type="configure">
             <type>yarn-site</type>
-            <transfer operation="COPY" 
from-key="yarn.timeline-service.leveldb-timeline-store.path" 
to-key="yarn.timeline-service.leveldb-state-store.path"/>
+            <transfer operation="copy" 
from-key="yarn.timeline-service.leveldb-timeline-store.path" 
to-key="yarn.timeline-service.leveldb-state-store.path"/>
           </task>
 
           <task xsi:type="configure">
@@ -579,7 +579,7 @@
           <task xsi:type="configure">
             <summary>Updating oozie-site to remove redundant 
configurations</summary>
             <type>oozie-site</type>
-            <transfer operation="DELETE" delete-key="*" preserve-edits="true">
+            <transfer operation="delete" delete-key="*" preserve-edits="true">
               <keep-key>oozie.base.url</keep-key>
               <keep-key>oozie.services.ext</keep-key>
               <keep-key>oozie.db.schema.name</keep-key>
@@ -615,13 +615,13 @@
         <pre-upgrade>
           <task xsi:type="configure">
             <type>oozie-site</type>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-job-submit-instances" 
to-key="oozie.service.ELService.ext.functions.coord-job-submit-instances"/>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-action-create-inst" 
to-key="oozie.service.ELService.ext.functions.coord-action-create-inst"/>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-action-create" 
to-key="oozie.service.ELService.ext.functions.coord-action-create"/>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-job-submit-data" 
to-key="oozie.service.ELService.ext.functions.coord-job-submit-data"/>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-action-start" 
to-key="oozie.service.ELService.ext.functions.coord-action-start"/>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-sla-submit" 
to-key="oozie.service.ELService.ext.functions.coord-sla-submit"/>
-            <transfer operation="COPY" 
from-key="oozie.service.ELService.ext.functions.coord-sla-create" 
to-key="oozie.service.ELService.ext.functions.coord-sla-create"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-job-submit-instances" 
to-key="oozie.service.ELService.ext.functions.coord-job-submit-instances"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-action-create-inst" 
to-key="oozie.service.ELService.ext.functions.coord-action-create-inst"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-action-create" 
to-key="oozie.service.ELService.ext.functions.coord-action-create"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-job-submit-data" 
to-key="oozie.service.ELService.ext.functions.coord-job-submit-data"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-action-start" 
to-key="oozie.service.ELService.ext.functions.coord-action-start"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-sla-submit" 
to-key="oozie.service.ELService.ext.functions.coord-sla-submit"/>
+            <transfer operation="copy" 
from-key="oozie.service.ELService.ext.functions.coord-sla-create" 
to-key="oozie.service.ELService.ext.functions.coord-sla-create"/>
           </task>
         </pre-upgrade>
         <upgrade>
@@ -657,9 +657,10 @@
           <task xsi:type="manual">
             <message>Before continuing, please deactivate and kill any 
currently running topologies.</message>
           </task>
-          <task xsi:type="configure" summary="Setting nimbus.seeds from 
nimbus.host">
+          <task xsi:type="configure" summary="Converting nimbus.host into 
nimbus.seeds">
             <type>storm-site</type>
-            <transfer operation="COPY" from-key="nimbus.host" 
to-key="nimbus.seeds" />
+            <transfer operation="copy" from-key="nimbus.host" 
to-key="nimbus.seeds" coerce-to="yaml-array" />
+            <transfer operation="delete" delete-key="nimbus.host" />
           </task>
         </pre-upgrade>
         <upgrade>

http://git-wip-us.apache.org/repos/asf/ambari/blob/59f15b2a/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 99882fd..50e0a17 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.TransferCoercionType;
 import org.apache.ambari.server.state.stack.upgrade.TransferOperation;
 import org.junit.After;
 import org.junit.Before;
@@ -363,6 +364,73 @@ public class ConfigureActionTest {
     assertTrue(map.containsKey("copyKey")); // is new
   }
 
+  @Test
+  public void testCoerceValueOnCopy() throws Exception {
+    makeUpgradeCluster();
+
+    Cluster c = m_injector.getInstance(Clusters.class).getCluster("c1");
+    assertEquals(1, c.getConfigsByType("zoo.cfg").size());
+
+    c.setDesiredStackVersion(HDP_21_STACK);
+    ConfigFactory cf = m_injector.getInstance(ConfigFactory.class);
+    Config config = cf.createNew(c, "zoo.cfg", new HashMap<String, String>() {
+      {
+        put("zoo.server.csv", "c6401,c6402,  c6403");
+      }
+    }, 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_1_0);
+    commandParams.put("clusterName", "c1");
+    commandParams.put(ConfigureTask.PARAMETER_CONFIG_TYPE, "zoo.cfg");
+
+    // copy with coerce
+    List<ConfigureTask.Transfer> transfers = new 
ArrayList<ConfigureTask.Transfer>();
+    ConfigureTask.Transfer transfer = new ConfigureTask.Transfer();
+    transfer.operation = TransferOperation.COPY;
+    transfer.coerceTo = TransferCoercionType.YAML_ARRAY;
+    transfer.fromKey = "zoo.server.csv";
+    transfer.toKey = "zoo.server.array";
+    transfer.defaultValue = "['foo','bar']";
+    transfers.add(transfer);
+
+    commandParams.put(ConfigureTask.PARAMETER_TRANSFERS, new 
Gson().toJson(transfers));
+
+    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");
+    assertNotNull(config);
+    assertFalse("version2".equals(config.getTag()));
+
+    Map<String, String> map = config.getProperties();
+    assertEquals("c6401,c6402,  c6403", map.get("zoo.server.csv"));
+    assertEquals("['c6401','c6402','c6403']", map.get("zoo.server.array"));
+  }
+
   private void makeUpgradeCluster() throws Exception {
     String clusterName = "c1";
     String hostName = "h1";

Reply via email to