HADOOP-7983. HA: failover should be able to pass args to fencers. Contributed by Eli Collins

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-1623@1238049 13f79535-47bb-0310-9956-ffa450edef68

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

@@ -34,3 +34,5 @@ HADOOP-7970. HAServiceProtocol methods must throw IOException.
 
 HADOOP-7992. Add ZKClient library to facilitate leader election.
 (Bikas Saha via suresh).
+
+HADOOP-7983. HA: failover should be able to pass args to fencers. (eli)

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

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.ha;
 
+import java.net.InetSocketAddress;
+
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.conf.Configurable;
@@ -52,6 +54,7 @@ public interface FenceMethod {
   
   /**
    * Attempt to fence the target node.
+   * @param serviceAddr the address (host:ipcport) of the service to fence
    * @param args the configured arguments, which were checked at startup by
    *             {@link #checkArgs(String)}
    * @return true if fencing was successful, false if unsuccessful or
@@ -59,5 +62,6 @@ public interface FenceMethod {
    * @throws BadFencingConfigurationException if the configuration was
    *         determined to be invalid only at runtime
    */
-  public boolean tryFence(String args) throws BadFencingConfigurationException; 
+  public boolean tryFence(InetSocketAddress serviceAddr, String args)
+    throws BadFencingConfigurationException;
 }

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

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.ha;
 
+import java.net.InetSocketAddress;
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -67,7 +68,7 @@ public class NodeFencer {
   private static final Log LOG = LogFactory.getLog(NodeFencer.class);
 
   /**
-   * Standard fencing methods included with HDFS.
+   * Standard fencing methods included with Hadoop.
    */
   private static final Map<String, Class<? extends FenceMethod>> STANDARD_METHODS =
     ImmutableMap.<String, Class<? extends FenceMethod>>of(
@@ -81,14 +82,14 @@ public class NodeFencer {
     this.methods = parseMethods(conf);
   }
   
-  public boolean fence() {
+  public boolean fence(InetSocketAddress serviceAddr) {
     LOG.info("====== Beginning NameNode Fencing Process... ======");
     int i = 0;
     for (FenceMethodWithArg method : methods) {
       LOG.info("Trying method " + (++i) + "/" + methods.size() +": " + method);
       
       try {
-        if (method.method.tryFence(method.arg)) {
+        if (method.method.tryFence(serviceAddr, method.arg)) {
           LOG.info("====== Fencing successful by method " + method + " ======");
           return true;
         }

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

@@ -19,11 +19,16 @@ 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;
 
@@ -70,9 +75,18 @@ public class ShellCommandFencer
   }
 
   @Override
-  public boolean tryFence(String cmd) {
+  public boolean tryFence(InetSocketAddress serviceAddr, String cmd) {
+    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", cmd);
+        "bash", "-e", "-c", cmdWithSvc);
     setConfAsEnvVars(builder.environment());
 
     Process p;

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

@@ -18,8 +18,7 @@
 package org.apache.hadoop.ha;
 
 import java.io.IOException;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
+import java.net.InetSocketAddress;
 import java.util.Collection;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -48,14 +47,9 @@ import com.jcraft.jsch.Session;
  * <p>
  * This fencing mechanism is configured as following in the fencing method
  * list:
- * <code>sshfence([username@]nnhost[:ssh-port], target-port)</code>
- * where the first argument specifies the username, host, and port to ssh
- * into, and the second argument specifies the port on which the target
- * NN process is listening on.
- * <p>
- * For example, <code>sshfence(other-nn, 8020)<code> will SSH into
- * <code>other-nn<code> as the current user on the standard SSH port,
- * then kill whatever process is listening on port 8020.
+ * <code>sshfence([[username][:ssh-port]])</code>
+ * where the optional argument specifies the username and port to use
+ * with ssh.
  * <p>
  * In order to achieve passwordless SSH, the operator must also configure
  * <code>dfs.namenode.ha.fencing.ssh.private-key-files<code> to point to an
@@ -75,25 +69,23 @@ public class SshFenceByTcpPort extends Configured
     "dfs.namenode.ha.fencing.ssh.private-key-files";
 
   /**
-   * Verify that the arguments are parseable and that the host
-   * can be resolved.
+   * Verify that the argument, if given, in the conf is parseable.
    */
   @Override
   public void checkArgs(String argStr) throws BadFencingConfigurationException {
-    Args args = new Args(argStr);
-    try {
-      InetAddress.getByName(args.host);
-    } catch (UnknownHostException e) {
-      throw new BadFencingConfigurationException(
-          "Unknown host: " + args.host);
+    if (argStr != null) {
+      // Use a dummy service when checking the arguments defined
+      // in the configuration are parseable.
+      Args args = new Args(new InetSocketAddress("localhost", 8020), argStr);
     }
   }
 
   @Override
-  public boolean tryFence(String argsStr)
+  public boolean tryFence(InetSocketAddress serviceAddr, String argsStr)
       throws BadFencingConfigurationException {
-    Args args = new Args(argsStr);
-    
+
+    Args args = new Args(serviceAddr, argsStr);
+
     Session session;
     try {
       session = createSession(args);
@@ -155,11 +147,11 @@ public class SshFenceByTcpPort extends Configured
             "Verifying whether it is running using nc...");
         rc = execCommand(session, "nc -z localhost 8020");
         if (rc == 0) {
-          // the NN is still listening - we are unable to fence
-          LOG.warn("Unable to fence NN - it is running but we cannot kill it");
+          // the service is still listening - we are unable to fence
+          LOG.warn("Unable to fence - it is running but we cannot kill it");
           return false;
         } else {
-          LOG.info("Verified that the NN is down.");
+          LOG.info("Verified that the service is down.");
           return true;          
         }
       } else {
@@ -189,7 +181,6 @@ public class SshFenceByTcpPort extends Configured
       exec.setCommand(cmd);
       exec.setInputStream(null);
       exec.connect();
-      
 
       // Pump stdout of the command to our WARN logs
       StreamPumper outPumper = new StreamPumper(LOG, cmd + " via ssh",
@@ -233,50 +224,37 @@ public class SshFenceByTcpPort extends Configured
    */
   @VisibleForTesting
   static class Args {
-    private static final Pattern USER_HOST_PORT_RE = Pattern.compile(
-      "(?:(.+?)@)?([^:]+?)(?:\\:(\\d+))?");
+    private static final Pattern USER_PORT_RE = Pattern.compile(
+      "([^:]+?)?(?:\\:(\\d+))?");
 
     private static final int DEFAULT_SSH_PORT = 22;
 
-    final String user;
-    final String host;
-    final int sshPort;
-    final int targetPort;
+    String host;
+    int targetPort;
+    String user;
+    int sshPort;
     
-    public Args(String args) throws BadFencingConfigurationException {
-      if (args == null) {
-        throw new BadFencingConfigurationException(
-            "Must specify args for ssh fencing configuration");
-      }
-      String[] argList = args.split(",\\s*");
-      if (argList.length != 2) {
-        throw new BadFencingConfigurationException(
-            "Incorrect number of arguments: " + args);
-      }
-      
-      // Parse SSH destination.
-      String sshDestArg = argList[0];
-      Matcher m = USER_HOST_PORT_RE.matcher(sshDestArg);
-      if (!m.matches()) {
-        throw new BadFencingConfigurationException(
-            "Unable to parse SSH destination: "+ sshDestArg);
-      }
-      if (m.group(1) != null) {
-        user = m.group(1);
-      } else {
-        user = System.getProperty("user.name");
-      }
-      
-      host = m.group(2);
+    public Args(InetSocketAddress serviceAddr, String arg) 
+        throws BadFencingConfigurationException {
+      host = serviceAddr.getHostName();
+      targetPort = serviceAddr.getPort();
+      user = System.getProperty("user.name");
+      sshPort = DEFAULT_SSH_PORT;
 
-      if (m.group(3) != null) {
-        sshPort = parseConfiggedPort(m.group(3));
-      } else {
-        sshPort = DEFAULT_SSH_PORT;
+      // Parse optional user and ssh port
+      if (arg != null && !"".equals(arg)) {
+        Matcher m = USER_PORT_RE.matcher(arg);
+        if (!m.matches()) {
+          throw new BadFencingConfigurationException(
+              "Unable to parse user and SSH port: "+ arg);
+        }
+        if (m.group(1) != null) {
+          user = m.group(1);
+        }
+        if (m.group(2) != null) {
+          sshPort = parseConfiggedPort(m.group(2));
+        }
       }
-      
-      // Parse target port.
-      targetPort = parseConfiggedPort(argList[1]);
     }
 
     private Integer parseConfiggedPort(String portStr)

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

@@ -19,6 +19,7 @@ package org.apache.hadoop.ha;
 
 import static org.junit.Assert.*;
 
+import java.net.InetSocketAddress;
 import java.util.List;
 
 import org.apache.hadoop.conf.Configuration;
@@ -42,8 +43,9 @@ public class TestNodeFencer {
   public void testSingleFencer() throws BadFencingConfigurationException {
     NodeFencer fencer = setupFencer(
         AlwaysSucceedFencer.class.getName() + "(foo)");
-    assertTrue(fencer.fence());
+    assertTrue(fencer.fence(new InetSocketAddress("host", 1234)));
     assertEquals(1, AlwaysSucceedFencer.fenceCalled);
+    assertEquals("host:1234", AlwaysSucceedFencer.fencedSvc);
     assertEquals("foo", AlwaysSucceedFencer.callArgs.get(0));
   }
   
@@ -52,7 +54,7 @@ public class TestNodeFencer {
     NodeFencer fencer = setupFencer(
         AlwaysSucceedFencer.class.getName() + "(foo)\n" +
         AlwaysSucceedFencer.class.getName() + "(bar)\n");
-    assertTrue(fencer.fence());
+    assertTrue(fencer.fence(new InetSocketAddress("host", 1234)));
     // Only one call, since the first fencer succeeds
     assertEquals(1, AlwaysSucceedFencer.fenceCalled);
     assertEquals("foo", AlwaysSucceedFencer.callArgs.get(0));
@@ -66,10 +68,12 @@ public class TestNodeFencer {
         " # the next one will always fail\n" +
         " " + AlwaysFailFencer.class.getName() + "(foo) # <- fails\n" +
         AlwaysSucceedFencer.class.getName() + "(bar) \n");
-    assertTrue(fencer.fence());
+    assertTrue(fencer.fence(new InetSocketAddress("host", 1234)));
     // One call to each, since top fencer fails
     assertEquals(1, AlwaysFailFencer.fenceCalled);
+    assertEquals("host:1234", AlwaysFailFencer.fencedSvc);
     assertEquals(1, AlwaysSucceedFencer.fenceCalled);
+    assertEquals("host:1234", AlwaysSucceedFencer.fencedSvc);
     assertEquals("foo", AlwaysFailFencer.callArgs.get(0));
     assertEquals("bar", AlwaysSucceedFencer.callArgs.get(0));
   }
@@ -78,18 +82,43 @@ public class TestNodeFencer {
   public void testArglessFencer() throws BadFencingConfigurationException {
     NodeFencer fencer = setupFencer(
         AlwaysSucceedFencer.class.getName());
-    assertTrue(fencer.fence());
+    assertTrue(fencer.fence(new InetSocketAddress("host", 1234)));
     // One call to each, since top fencer fails
     assertEquals(1, AlwaysSucceedFencer.fenceCalled);
+    assertEquals("host:1234", AlwaysSucceedFencer.fencedSvc);
     assertEquals(null, AlwaysSucceedFencer.callArgs.get(0));
   }
-  
+
   @Test
-  public void testShortName() throws BadFencingConfigurationException {
+  public void testShortNameShell() throws BadFencingConfigurationException {
     NodeFencer fencer = setupFencer("shell(true)");
-    assertTrue(fencer.fence());
+    assertTrue(fencer.fence(new InetSocketAddress("host", 1234)));
   }
- 
+
+  @Test
+  public void testShortNameSsh() throws BadFencingConfigurationException {
+    NodeFencer fencer = setupFencer("sshfence");
+    assertFalse(fencer.fence(new InetSocketAddress("host", 1234)));
+  }
+
+  @Test
+  public void testShortNameSshWithUser() throws BadFencingConfigurationException {
+    NodeFencer fencer = setupFencer("sshfence(user)");
+    assertFalse(fencer.fence(new InetSocketAddress("host", 1234)));
+  }
+
+  @Test
+  public void testShortNameSshWithPort() throws BadFencingConfigurationException {
+    NodeFencer fencer = setupFencer("sshfence(:123)");
+    assertFalse(fencer.fence(new InetSocketAddress("host", 1234)));
+  }
+
+  @Test
+  public void testShortNameSshWithUserPort() throws BadFencingConfigurationException {
+    NodeFencer fencer = setupFencer("sshfence(user:123)");
+    assertFalse(fencer.fence(new InetSocketAddress("host", 1234)));
+  }
+
   private NodeFencer setupFencer(String confStr)
       throws BadFencingConfigurationException {
     System.err.println("Testing configuration:\n" + confStr);
@@ -105,10 +134,12 @@ public class TestNodeFencer {
   public static class AlwaysSucceedFencer extends Configured
       implements FenceMethod {
     static int fenceCalled = 0;
+    static String fencedSvc;
     static List<String> callArgs = Lists.newArrayList();
 
     @Override
-    public boolean tryFence(String args) {
+    public boolean tryFence(InetSocketAddress serviceAddr, String args) {
+      fencedSvc = serviceAddr.getHostName() + ":" + serviceAddr.getPort();
       callArgs.add(args);
       fenceCalled++;
       return true;
@@ -125,10 +156,12 @@ public class TestNodeFencer {
   public static class AlwaysFailFencer extends Configured
       implements FenceMethod {
     static int fenceCalled = 0;
+    static String fencedSvc;
     static List<String> callArgs = Lists.newArrayList();
 
     @Override
-    public boolean tryFence(String args) {
+    public boolean tryFence(InetSocketAddress serviceAddr, String args) {
+      fencedSvc = serviceAddr.getHostName() + ":" + serviceAddr.getPort();
       callArgs.add(args);
       fenceCalled++;
       return false;

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

@@ -19,6 +19,8 @@ package org.apache.hadoop.ha;
 
 import static org.junit.Assert.*;
 
+import java.net.InetSocketAddress;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.util.StringUtils;
 import org.junit.Before;
@@ -55,14 +57,15 @@ public class TestShellCommandFencer {
    */
   @Test
   public void testBasicSuccessFailure() {
-    assertTrue(fencer.tryFence("exit 0"));
-    assertFalse(fencer.tryFence("exit 1"));
+    InetSocketAddress addr = new InetSocketAddress("host", 1234);
+    assertTrue(fencer.tryFence(addr, "echo"));
+    assertFalse(fencer.tryFence(addr, "exit 1"));
     // bad path should also fail
-    assertFalse(fencer.tryFence("xxxxxxxxxxxx"));
+    assertFalse(fencer.tryFence(addr, "xxxxxxxxxxxx"));
   }
   
   @Test
-  public void testCheckArgs() {
+  public void testCheckNoArgs() {
     try {
       Configuration conf = new Configuration();
       conf.set(NodeFencer.CONF_METHODS_KEY, "shell");
@@ -74,16 +77,31 @@ public class TestShellCommandFencer {
         confe.getMessage().contains("No argument passed"));    
     }
   }
-  
+
+  @Test
+  public void testCheckParensNoArgs() {
+    try {
+      Configuration conf = new Configuration();
+      conf.set(NodeFencer.CONF_METHODS_KEY, "shell()");
+      new NodeFencer(conf);
+      fail("Didn't throw when passing no args to shell");
+    } catch (BadFencingConfigurationException confe) {
+      assertTrue(
+        "Unexpected exception:" + StringUtils.stringifyException(confe),
+        confe.getMessage().contains("Unable to parse line: 'shell()'"));
+    }
+  }
+
   /**
    * Test that lines on stdout get passed as INFO
    * level messages
    */
   @Test
   public void testStdoutLogging() {
-    assertTrue(fencer.tryFence("echo hello"));
+    InetSocketAddress addr = new InetSocketAddress("host", 1234);
+    assertTrue(fencer.tryFence(addr, "echo hello"));
     Mockito.verify(ShellCommandFencer.LOG).info(
-        Mockito.endsWith("echo hello: hello"));
+        Mockito.endsWith("echo hello: host:1234 hello"));
   }
    
   /**
@@ -92,9 +110,10 @@ public class TestShellCommandFencer {
    */
   @Test
   public void testStderrLogging() {
-    assertTrue(fencer.tryFence("echo hello >&2"));
+    InetSocketAddress addr = new InetSocketAddress("host", 1234);
+    assertTrue(fencer.tryFence(addr, "echo hello >&2"));
     Mockito.verify(ShellCommandFencer.LOG).warn(
-        Mockito.endsWith("echo hello >&2: hello"));
+        Mockito.endsWith("echo hello >&2: host:1234 hello"));
   }
 
   /**
@@ -103,9 +122,10 @@ public class TestShellCommandFencer {
    */
   @Test
   public void testConfAsEnvironment() {
-    fencer.tryFence("echo $in_fencing_tests");
+    InetSocketAddress addr = new InetSocketAddress("host", 1234);
+    fencer.tryFence(addr, "echo $in_fencing_tests");
     Mockito.verify(ShellCommandFencer.LOG).info(
-        Mockito.endsWith("echo $in...ing_tests: yessir"));
+        Mockito.endsWith("echo $in...ing_tests: host:1234 yessir"));
   }
 
   /**
@@ -116,7 +136,8 @@ public class TestShellCommandFencer {
    */
   @Test(timeout=10000)
   public void testSubprocessInputIsClosed() {
-    assertFalse(fencer.tryFence("read"));
+    InetSocketAddress addr = new InetSocketAddress("host", 1234);
+    assertFalse(fencer.tryFence(addr, "read"));
   }
   
   @Test

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

@@ -19,9 +19,10 @@ package org.apache.hadoop.ha;
 
 import static org.junit.Assert.*;
 
+import java.net.InetSocketAddress;
+
 import org.apache.commons.logging.impl.Log4JLogger;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.ha.SshFenceByTcpPort.Args;
 import org.apache.log4j.Level;
 import org.junit.Assume;
@@ -33,8 +34,10 @@ public class TestSshFenceByTcpPort {
     ((Log4JLogger)SshFenceByTcpPort.LOG).getLogger().setLevel(Level.ALL);
   }
   
-  private String TEST_FENCING_ARG = System.getProperty(
-      "test.TestSshFenceByTcpPort.arg", "localhost");
+  private String TEST_FENCING_HOST = System.getProperty(
+      "test.TestSshFenceByTcpPort.host", "localhost");
+  private String TEST_FENCING_PORT = System.getProperty(
+      "test.TestSshFenceByTcpPort.port", "8020");
   private final String TEST_KEYFILE = System.getProperty(
       "test.TestSshFenceByTcpPort.key");
 
@@ -43,10 +46,12 @@ public class TestSshFenceByTcpPort {
     Assume.assumeTrue(isConfigured());
     Configuration conf = new Configuration();
     conf.set(SshFenceByTcpPort.CONF_IDENTITIES_KEY, TEST_KEYFILE);
-    FileSystem.setDefaultUri(conf, "localhost:8020");
     SshFenceByTcpPort fence = new SshFenceByTcpPort();
     fence.setConf(conf);
-    assertTrue(fence.tryFence(TEST_FENCING_ARG));
+    assertTrue(fence.tryFence(
+        new InetSocketAddress(TEST_FENCING_HOST,
+                              Integer.valueOf(TEST_FENCING_PORT)),
+        null));
   }
 
   /**
@@ -61,61 +66,65 @@ public class TestSshFenceByTcpPort {
     SshFenceByTcpPort fence = new SshFenceByTcpPort();
     fence.setConf(conf);
     // Connect to Google's DNS server - not running ssh!
-    assertFalse(fence.tryFence("8.8.8.8, 1234"));
+    assertFalse(fence.tryFence(new InetSocketAddress("8.8.8.8", 1234), ""));
   }
   
   @Test
   public void testArgsParsing() throws BadFencingConfigurationException {
-    Args args = new SshFenceByTcpPort.Args("foo@bar.com:1234, 5678");
-    assertEquals("foo", args.user);
-    assertEquals("bar.com", args.host);
-    assertEquals(1234, args.sshPort);
-    assertEquals(5678, args.targetPort);
+    InetSocketAddress addr = new InetSocketAddress("bar.com", 1234);
 
-    args = new SshFenceByTcpPort.Args("foo@bar.com, 1234");
-    assertEquals("foo", args.user);
+    Args args = new SshFenceByTcpPort.Args(addr, null);
     assertEquals("bar.com", args.host);
-    assertEquals(22, args.sshPort);
     assertEquals(1234, args.targetPort);
+    assertEquals(System.getProperty("user.name"), args.user);
+    assertEquals(22, args.sshPort);
     
-    args = new SshFenceByTcpPort.Args("bar.com, 1234");
+    args = new SshFenceByTcpPort.Args(addr, "");
+    assertEquals("bar.com", args.host);
+    assertEquals(1234, args.targetPort);    
     assertEquals(System.getProperty("user.name"), args.user);
+    assertEquals(22, args.sshPort);
+
+    args = new SshFenceByTcpPort.Args(addr, "12345");
     assertEquals("bar.com", args.host);
+    assertEquals(1234, args.targetPort);
+    assertEquals("12345", args.user);
     assertEquals(22, args.sshPort);
+
+    args = new SshFenceByTcpPort.Args(addr, ":12345");
+    assertEquals("bar.com", args.host);
     assertEquals(1234, args.targetPort);
-    
-    args = new SshFenceByTcpPort.Args("bar.com:1234, 12345");
     assertEquals(System.getProperty("user.name"), args.user);
+    assertEquals(12345, args.sshPort);
+
+    args = new SshFenceByTcpPort.Args(addr, "foo:8020");
     assertEquals("bar.com", args.host);
-    assertEquals(1234, args.sshPort);
-    assertEquals(12345, args.targetPort);
-    
-    args = new SshFenceByTcpPort.Args("bar, 8020");
-    assertEquals(8020, args.targetPort);    
+    assertEquals(1234, args.targetPort);
+    assertEquals("foo", args.user);
+    assertEquals(8020, args.sshPort);
   }
   
   @Test
   public void testBadArgsParsing() throws BadFencingConfigurationException {
-    assertBadArgs(null);
-    assertBadArgs("");
-    assertBadArgs("bar.com:");
-    assertBadArgs("bar.com:x");
-    assertBadArgs("foo.com, x");
-    assertBadArgs("foo.com,");
-    assertBadArgs("foo.com, ");
+    assertBadArgs(":");          // No port specified
+    assertBadArgs("bar.com:");   // "
+    assertBadArgs(":xx");        // Port does not parse
+    assertBadArgs("bar.com:xx"); // "
   }
   
   private void assertBadArgs(String argStr) {
+    InetSocketAddress addr = new InetSocketAddress("bar.com", 1234);
     try {
-      new Args(argStr);
+      new Args(addr, argStr);
       fail("Did not fail on bad args: " + argStr);
     } catch (BadFencingConfigurationException e) {
-      // expected
+      // Expected
     }
   }
 
   private boolean isConfigured() {
-    return (TEST_FENCING_ARG != null && !TEST_FENCING_ARG.isEmpty()) &&
-      (TEST_KEYFILE != null && !TEST_KEYFILE.isEmpty());
+    return (TEST_FENCING_HOST != null && !TEST_FENCING_HOST.isEmpty()) &&
+           (TEST_FENCING_PORT != null && !TEST_FENCING_PORT.isEmpty()) &&
+           (TEST_KEYFILE != null && !TEST_KEYFILE.isEmpty());
   }
 }