HADOOP-8007. Use substitution tokens for fencing argument. Contributed by Todd Lipcon.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1309284 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -134,6 +134,8 @@ Release 2.0.0 - UNRELEASED
     HADOOP-8242. AbstractDelegationTokenIdentifier: add getter methods
     for owner and realuser. (Colin Patrick McCabe via eli)
 
+    HADOOP-8007. Use substitution tokens for fencing argument (todd)
+
   OPTIMIZATIONS
 
   BUG FIXES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAServiceTarget.java

@@ -19,6 +19,7 @@ package org.apache.hadoop.ha;
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
+import java.util.Map;
 
 import javax.net.SocketFactory;
 
@@ -29,6 +30,8 @@ import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
 import org.apache.hadoop.ha.protocolPB.HAServiceProtocolClientSideTranslatorPB;
 import org.apache.hadoop.net.NetUtils;
 
+import com.google.common.collect.Maps;
+
 /**
  * Represents a target of the client side HA administration commands.
  */
@@ -36,6 +39,10 @@ import org.apache.hadoop.net.NetUtils;
 @InterfaceStability.Evolving
 public abstract class HAServiceTarget {
 
+  private static final String HOST_SUBST_KEY = "host";
+  private static final String PORT_SUBST_KEY = "port";
+  private static final String ADDRESS_SUBST_KEY = "address";
+
   /**
    * @return the IPC address of the target node.
    */
@@ -68,4 +75,28 @@ public abstract class HAServiceTarget {
         getAddress(),
         confCopy, factory, timeoutMs);
   }
+  
+  public final Map<String, String> getFencingParameters() {
+    Map<String, String> ret = Maps.newHashMap();
+    addFencingParameters(ret);
+    return ret;
+  }
+  
+  /**
+   * Hook to allow subclasses to add any parameters they would like to
+   * expose to fencing implementations/scripts. Fencing methods are free
+   * to use this map as they see fit -- notably, the shell script
+   * implementation takes each entry, prepends 'target_', substitutes
+   * '_' for '.', and adds it to the environment of the script.
+   *
+   * Subclass implementations should be sure to delegate to the superclass
+   * implementation as well as adding their own keys.
+   *
+   * @param ret map which can be mutated to pass parameters to the fencer
+   */
+  protected void addFencingParameters(Map<String, String> ret) {
+    ret.put(ADDRESS_SUBST_KEY, String.valueOf(getAddress()));
+    ret.put(HOST_SUBST_KEY, getAddress().getHostName());
+    ret.put(PORT_SUBST_KEY, String.valueOf(getAddress().getPort()));
+  }
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java

@@ -19,16 +19,11 @@ package org.apache.hadoop.ha;
 
 import java.io.IOException;
 import java.lang.reflect.Field;
-import java.net.InetSocketAddress;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
 import java.util.Map;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configured;
-import org.apache.hadoop.util.StringUtils;
 
 import com.google.common.annotations.VisibleForTesting;
 
@@ -61,6 +56,9 @@ public class ShellCommandFencer
   /** Length at which to abbreviate command in long messages */
   private static final int ABBREV_LENGTH = 20;
 
+  /** Prefix for target parameters added to the environment */
+  private static final String TARGET_PREFIX = "target_";
+  
   @VisibleForTesting
   static Log LOG = LogFactory.getLog(
       ShellCommandFencer.class);
@@ -76,19 +74,10 @@ public class ShellCommandFencer
 
   @Override
   public boolean tryFence(HAServiceTarget target, String cmd) {
-    InetSocketAddress serviceAddr = target.getAddress();
-    List<String> cmdList = Arrays.asList(cmd.split("\\s+"));
-
-    // Create arg list with service as the first argument
-    List<String> argList = new ArrayList<String>();
-    argList.add(cmdList.get(0));
-    argList.add(serviceAddr.getHostName() + ":" + serviceAddr.getPort());
-    argList.addAll(cmdList.subList(1, cmdList.size()));
-    String cmdWithSvc = StringUtils.join(" ", argList);
-
     ProcessBuilder builder = new ProcessBuilder(
-        "bash", "-e", "-c", cmdWithSvc);
+        "bash", "-e", "-c", cmd);
     setConfAsEnvVars(builder.environment());
+    addTargetInfoAsEnvVars(target, builder.environment());
 
     Process p;
     try {
@@ -185,4 +174,21 @@ public class ShellCommandFencer
       env.put(pair.getKey().replace('.', '_'), pair.getValue());
     }
   }
+
+  /**
+   * Add information about the target to the the environment of the
+   * subprocess.
+   * 
+   * @param target
+   * @param environment
+   */
+  private void addTargetInfoAsEnvVars(HAServiceTarget target,
+      Map<String, String> environment) {
+    for (Map.Entry<String, String> e :
+         target.getFencingParameters().entrySet()) {
+      String key = TARGET_PREFIX + e.getKey();
+      key = key.replace('.', '_');
+      environment.put(key, e.getValue());
+    }
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java

@@ -103,7 +103,7 @@ public class TestShellCommandFencer {
   public void testStdoutLogging() {
     assertTrue(fencer.tryFence(TEST_TARGET, "echo hello"));
     Mockito.verify(ShellCommandFencer.LOG).info(
-        Mockito.endsWith("echo hello: host:1234 hello"));
+        Mockito.endsWith("echo hello: hello"));
   }
    
   /**
@@ -114,7 +114,7 @@ public class TestShellCommandFencer {
   public void testStderrLogging() {
     assertTrue(fencer.tryFence(TEST_TARGET, "echo hello >&2"));
     Mockito.verify(ShellCommandFencer.LOG).warn(
-        Mockito.endsWith("echo hello >&2: host:1234 hello"));
+        Mockito.endsWith("echo hello >&2: hello"));
   }
 
   /**
@@ -125,9 +125,21 @@ public class TestShellCommandFencer {
   public void testConfAsEnvironment() {
     fencer.tryFence(TEST_TARGET, "echo $in_fencing_tests");
     Mockito.verify(ShellCommandFencer.LOG).info(
-        Mockito.endsWith("echo $in...ing_tests: host:1234 yessir"));
+        Mockito.endsWith("echo $in...ing_tests: yessir"));
   }
   
+  /**
+   * Verify that information about the fencing target gets passed as
+   * environment variables to the fencer.
+   */
+  @Test
+  public void testTargetAsEnvironment() {
+    fencer.tryFence(TEST_TARGET, "echo $target_host $target_port $target_address");
+    Mockito.verify(ShellCommandFencer.LOG).info(
+        Mockito.endsWith("echo $ta...t_address: host 1234 host:1234"));
+  }
+
+
   /**
    * Test that we properly close off our input to the subprocess
    * such that it knows there's no tty connected. This is important