diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 22d89d9d1bb..0c03d99da51 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/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 diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAServiceTarget.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAServiceTarget.java index 90c887d2923..00edfa0d8ba 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAServiceTarget.java +++ b/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 getFencingParameters() { + Map 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 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())); + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java index db5676dfc0e..c8b7a30dd13 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java +++ b/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; @@ -60,6 +55,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( @@ -76,19 +74,10 @@ public class ShellCommandFencer @Override public boolean tryFence(HAServiceTarget target, String cmd) { - InetSocketAddress serviceAddr = target.getAddress(); - List cmdList = Arrays.asList(cmd.split("\\s+")); - - // Create arg list with service as the first argument - List argList = new ArrayList(); - 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 environment) { + for (Map.Entry e : + target.getFencingParameters().entrySet()) { + String key = TARGET_PREFIX + e.getKey(); + key = key.replace('.', '_'); + environment.put(key, e.getValue()); + } + } +} \ No newline at end of file diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java index e95ba59af27..380f2512648 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java +++ b/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,8 +125,20 @@ 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