diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java index 61da7e926d9..9d03a4485be 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java @@ -22,8 +22,10 @@ import java.net.InetSocketAddress; import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import org.apache.hadoop.classification.InterfaceAudience.Private; import org.apache.hadoop.conf.Configuration; @@ -138,9 +140,10 @@ public class RouterAdmin extends Configured implements Tool { + "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + "-owner -group -mode ]"; } else if (cmd.equals("-update")) { - return "\t[-update " - + " " - + "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + return "\t[-update " + + " [ ] " + + "[-readonly true|false] [-faulttolerant true|false]" + + " [-order HASH|LOCAL|RANDOM|HASH_ALL] " + "-owner -group -mode ]"; } else if (cmd.equals("-rm")) { return "\t[-rm ]"; @@ -294,6 +297,8 @@ public class RouterAdmin extends Configured implements Tool { } else if ("-update".equals(cmd)) { if (updateMount(argv, i)) { System.out.println("Successfully updated mount point " + argv[i]); + System.out.println( + "WARN: Changing order/destinations may lead to inconsistencies"); } else { exitCode = -1; } @@ -366,6 +371,10 @@ public class RouterAdmin extends Configured implements Tool { e.printStackTrace(); debugException = ex; } + } catch (IOException ioe) { + exitCode = -1; + System.err.println(cmd.substring(1) + ": " + ioe.getLocalizedMessage()); + printUsage(cmd); } catch (Exception e) { exitCode = -1; debugException = e; @@ -473,17 +482,7 @@ public class RouterAdmin extends Configured implements Tool { mount = normalizeFileSystemPath(mount); // Get the existing entry MountTableManager mountTable = client.getMountTableManager(); - GetMountTableEntriesRequest getRequest = - GetMountTableEntriesRequest.newInstance(mount); - GetMountTableEntriesResponse getResponse = - mountTable.getMountTableEntries(getRequest); - List results = getResponse.getEntries(); - MountTable existingEntry = null; - for (MountTable result : results) { - if (mount.equals(result.getSourcePath())) { - existingEntry = result; - } - } + MountTable existingEntry = getMountEntry(mount, mountTable); if (existingEntry == null) { // Create and add the entry if it doesn't exist @@ -579,100 +578,81 @@ public class RouterAdmin extends Configured implements Tool { * @throws IOException If there is an error. */ public boolean updateMount(String[] parameters, int i) throws IOException { - // Mandatory parameters String mount = parameters[i++]; - String[] nss = parameters[i++].split(","); - String dest = parameters[i++]; - - // Optional parameters - boolean readOnly = false; - boolean faultTolerant = false; - String owner = null; - String group = null; - FsPermission mode = null; - DestinationOrder order = null; - while (i < parameters.length) { - if (parameters[i].equals("-readonly")) { - readOnly = true; - } else if (parameters[i].equals("-faulttolerant")) { - faultTolerant = true; - } else if (parameters[i].equals("-order")) { - i++; - try { - order = DestinationOrder.valueOf(parameters[i]); - } catch(Exception e) { - System.err.println("Cannot parse order: " + parameters[i]); - } - } else if (parameters[i].equals("-owner")) { - i++; - owner = parameters[i]; - } else if (parameters[i].equals("-group")) { - i++; - group = parameters[i]; - } else if (parameters[i].equals("-mode")) { - i++; - short modeValue = Short.parseShort(parameters[i], 8); - mode = new FsPermission(modeValue); - } else { - printUsage("-update"); - return false; - } - - i++; - } - - return updateMount(mount, nss, dest, readOnly, faultTolerant, order, - new ACLEntity(owner, group, mode)); - } - - /** - * Update a mount table entry. - * - * @param mount Mount point. - * @param nss Nameservices where this is mounted to. - * @param dest Destination path. - * @param readonly If the mount point is read only. - * @param order Order of the destination locations. - * @param aclInfo the ACL info for mount point. - * @return If the mount point was updated. - * @throws IOException Error updating the mount point. - */ - public boolean updateMount(String mount, String[] nss, String dest, - boolean readonly, boolean faultTolerant, - DestinationOrder order, ACLEntity aclInfo) - throws IOException { mount = normalizeFileSystemPath(mount); MountTableManager mountTable = client.getMountTableManager(); - - // Create a new entry - Map destMap = new LinkedHashMap<>(); - for (String ns : nss) { - destMap.put(ns, dest); + MountTable existingEntry = getMountEntry(mount, mountTable); + if (existingEntry == null) { + throw new IOException(mount + " doesn't exist."); } - MountTable newEntry = MountTable.newInstance(mount, destMap); + // Check if the destination needs to be updated. - newEntry.setReadOnly(readonly); - newEntry.setFaultTolerant(faultTolerant); - - if (order != null) { - newEntry.setDestOrder(order); + if (!parameters[i].startsWith("-")) { + String[] nss = parameters[i++].split(","); + String dest = parameters[i++]; + Map destMap = new LinkedHashMap<>(); + for (String ns : nss) { + destMap.put(ns, dest); + } + final List locations = new LinkedList<>(); + for (Entry entry : destMap.entrySet()) { + String nsId = entry.getKey(); + String path = normalizeFileSystemPath(entry.getValue()); + RemoteLocation location = new RemoteLocation(nsId, path, mount); + locations.add(location); + } + existingEntry.setDestinations(locations); } - - // Update ACL info of mount table entry - if (aclInfo.getOwner() != null) { - newEntry.setOwnerName(aclInfo.getOwner()); + try { + while (i < parameters.length) { + switch (parameters[i]) { + case "-readonly": + i++; + existingEntry.setReadOnly(getBooleanValue(parameters[i])); + break; + case "-faulttolerant": + i++; + existingEntry.setFaultTolerant(getBooleanValue(parameters[i])); + break; + case "-order": + i++; + try { + existingEntry.setDestOrder(DestinationOrder.valueOf(parameters[i])); + break; + } catch (Exception e) { + throw new Exception("Cannot parse order: " + parameters[i]); + } + case "-owner": + i++; + existingEntry.setOwnerName(parameters[i]); + break; + case "-group": + i++; + existingEntry.setGroupName(parameters[i]); + break; + case "-mode": + i++; + short modeValue = Short.parseShort(parameters[i], 8); + existingEntry.setMode(new FsPermission(modeValue)); + break; + default: + printUsage("-update"); + return false; + } + i++; + } + } catch (IllegalArgumentException iae) { + throw iae; + } catch (Exception e) { + String msg = "Unable to parse arguments: " + e.getMessage(); + if (e instanceof ArrayIndexOutOfBoundsException) { + msg = "Unable to parse arguments: no value provided for " + + parameters[i - 1]; + } + throw new IOException(msg); } - - if (aclInfo.getGroup() != null) { - newEntry.setGroupName(aclInfo.getGroup()); - } - - if (aclInfo.getMode() != null) { - newEntry.setMode(aclInfo.getMode()); - } - UpdateMountTableEntryRequest updateRequest = - UpdateMountTableEntryRequest.newInstance(newEntry); + UpdateMountTableEntryRequest.newInstance(existingEntry); UpdateMountTableEntryResponse updateResponse = mountTable.updateMountTableEntry(updateRequest); boolean updated = updateResponse.getStatus(); @@ -682,6 +662,45 @@ public class RouterAdmin extends Configured implements Tool { return updated; } + /** + * Parse string to boolean. + * @param value the string to be parsed. + * @return parsed boolean value. + * @throws Exception if other than true|false is provided. + */ + private boolean getBooleanValue(String value) throws Exception { + if (value.equalsIgnoreCase("true")) { + return true; + } else if (value.equalsIgnoreCase("false")) { + return false; + } + throw new IllegalArgumentException("Invalid argument: " + value + + ". Please specify either true or false."); + } + + /** + * Gets the mount table entry. + * @param mount name of the mount entry. + * @param mountTable the mount table. + * @return corresponding mount entry. + * @throws IOException in case of failure to retrieve mount entry. + */ + private MountTable getMountEntry(String mount, MountTableManager mountTable) + throws IOException { + GetMountTableEntriesRequest getRequest = + GetMountTableEntriesRequest.newInstance(mount); + GetMountTableEntriesResponse getResponse = + mountTable.getMountTableEntries(getRequest); + List results = getResponse.getEntries(); + MountTable existingEntry = null; + for (MountTable result : results) { + if (mount.equals(result.getSourcePath())) { + existingEntry = result; + } + } + return existingEntry; + } + /** * Remove mount point. * diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java index 381203b2a91..ce260ec0976 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java @@ -536,8 +536,8 @@ public class TestRouterAdminCLI { argv = new String[] {"-update", src, nsId}; assertEquals(-1, ToolRunner.run(admin, argv)); assertTrue("Wrong message: " + out, out.toString().contains( - "\t[-update " - + "[-readonly] [-faulttolerant] " + "\t[-update [ ] " + + "[-readonly true|false] [-faulttolerant true|false] " + "[-order HASH|LOCAL|RANDOM|HASH_ALL] " + "-owner -group -mode ]")); out.reset(); @@ -581,9 +581,9 @@ public class TestRouterAdminCLI { + "\t[-add " + "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + "-owner -group -mode ]\n" - + "\t[-update " - + " " - + "[-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "\t[-update [ " + + "] [-readonly true|false]" + + " [-faulttolerant true|false] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + "-owner -group -mode ]\n" + "\t[-rm ]\n" + "\t[-ls ]\n" + "\t[-getDestination ]\n" @@ -902,23 +902,15 @@ public class TestRouterAdminCLI { @Test public void testUpdateNonExistingMountTable() throws Exception { - System.setOut(new PrintStream(out)); + System.setErr(new PrintStream(err)); String nsId = "ns0"; String src = "/test-updateNonExistingMounttable"; String dest = "/updateNonExistingMounttable"; String[] argv = new String[] {"-update", src, nsId, dest}; - assertEquals(0, ToolRunner.run(admin, argv)); - - stateStore.loadCache(MountTableStoreImpl.class, true); - GetMountTableEntriesRequest getRequest = - GetMountTableEntriesRequest.newInstance(src); - GetMountTableEntriesResponse getResponse = - client.getMountTableManager().getMountTableEntries(getRequest); - // Ensure the destination updated successfully - MountTable mountTable = getResponse.getEntries().get(0); - assertEquals(src, mountTable.getSourcePath()); - assertEquals(nsId, mountTable.getDestinations().get(0).getNameserviceId()); - assertEquals(dest, mountTable.getDestinations().get(0).getDest()); + // Update shall fail if the mount entry doesn't exist. + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(err.toString(), err.toString() + .contains("update: /test-updateNonExistingMounttable doesn't exist.")); } @Test @@ -997,6 +989,106 @@ public class TestRouterAdminCLI { assertEquals(newDest, mountTable.getDestinations().get(0).getDest()); } + @Test + public void testUpdateChangeAttributes() throws Exception { + // Add a mount table firstly + String nsId = "ns0"; + String src = "/mount"; + String dest = "/dest"; + String[] argv = new String[] {"-add", src, nsId, dest, "-readonly", + "-order", "HASH_ALL"}; + assertEquals(0, ToolRunner.run(admin, argv)); + + stateStore.loadCache(MountTableStoreImpl.class, true); + GetMountTableEntriesRequest getRequest = + GetMountTableEntriesRequest.newInstance(src); + GetMountTableEntriesResponse getResponse = + client.getMountTableManager().getMountTableEntries(getRequest); + // Ensure mount table added successfully + MountTable mountTable = getResponse.getEntries().get(0); + assertEquals(src, mountTable.getSourcePath()); + + // Update the destination + String newNsId = "ns0"; + String newDest = "/newDestination"; + argv = new String[] {"-update", src, newNsId, newDest}; + assertEquals(0, ToolRunner.run(admin, argv)); + + stateStore.loadCache(MountTableStoreImpl.class, true); + getResponse = + client.getMountTableManager().getMountTableEntries(getRequest); + // Ensure the destination updated successfully and other attributes are + // preserved. + mountTable = getResponse.getEntries().get(0); + assertEquals(src, mountTable.getSourcePath()); + assertEquals(newNsId, + mountTable.getDestinations().get(0).getNameserviceId()); + assertEquals(newDest, mountTable.getDestinations().get(0).getDest()); + assertTrue(mountTable.isReadOnly()); + assertEquals("HASH_ALL", mountTable.getDestOrder().toString()); + + // Update the attribute. + argv = new String[] {"-update", src, "-readonly", "false"}; + assertEquals(0, ToolRunner.run(admin, argv)); + + stateStore.loadCache(MountTableStoreImpl.class, true); + getResponse = + client.getMountTableManager().getMountTableEntries(getRequest); + + // Ensure the attribute updated successfully and destination and other + // attributes are preserved. + mountTable = getResponse.getEntries().get(0); + assertEquals(src, mountTable.getSourcePath()); + assertEquals(newNsId, + mountTable.getDestinations().get(0).getNameserviceId()); + assertEquals(newDest, mountTable.getDestinations().get(0).getDest()); + assertFalse(mountTable.isReadOnly()); + assertEquals("HASH_ALL", mountTable.getDestOrder().toString()); + + } + + @Test + public void testUpdateErrorCase() throws Exception { + // Add a mount table firstly + String nsId = "ns0"; + String src = "/mount"; + String dest = "/dest"; + String[] argv = new String[] {"-add", src, nsId, dest, "-readonly", + "-order", "HASH_ALL"}; + assertEquals(0, ToolRunner.run(admin, argv)); + stateStore.loadCache(MountTableStoreImpl.class, true); + + // Check update for non-existent mount entry. + argv = new String[] {"-update", "/noMount", "-readonly", "false"}; + System.setErr(new PrintStream(err)); + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(err.toString(), + err.toString().contains("update: /noMount doesn't exist.")); + err.reset(); + + // Check update if non true/false value is passed for readonly. + argv = new String[] {"-update", src, "-readonly", "check"}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(err.toString(), err.toString().contains("update: " + + "Invalid argument: check. Please specify either true or false.")); + err.reset(); + + // Check update with missing value is passed for faulttolerant. + argv = new String[] {"-update", src, "ns1", "/tmp", "-faulttolerant"}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(err.toString(), + err.toString().contains("update: Unable to parse arguments:" + + " no value provided for -faulttolerant")); + err.reset(); + + // Check update with invalid order. + argv = new String[] {"-update", src, "ns1", "/tmp", "-order", "Invalid"}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(err.toString(), err.toString().contains( + "update: Unable to parse arguments: Cannot parse order: Invalid")); + err.reset(); + } + @Test public void testUpdateReadonlyUserGroupPermissionMountable() throws Exception { @@ -1022,7 +1114,7 @@ public class TestRouterAdminCLI { // Update the readonly, owner, group and permission String testOwner = "test_owner"; String testGroup = "test_group"; - argv = new String[] {"-update", src, nsId, dest, "-readonly", + argv = new String[] {"-update", src, nsId, dest, "-readonly", "true", "-owner", testOwner, "-group", testGroup, "-mode", "0455"}; assertEquals(0, ToolRunner.run(admin, argv)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md b/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md index 452b2773698..fd77edf754d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md +++ b/hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/HDFSCommands.md @@ -429,7 +429,7 @@ Usage: hdfs dfsrouteradmin [-add [-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] -owner -group -mode ] - [-update [-readonly] [-faulttolerant] [-order HASH|LOCAL|RANDOM|HASH_ALL] -owner -group -mode ] + [-update [ ] [-readonly true|false] [-faulttolerant true|false] [-order HASH|LOCAL|RANDOM|HASH_ALL] -owner -group -mode ] [-rm ] [-ls ] [-getDestination ] @@ -444,7 +444,7 @@ Usage: | COMMAND\_OPTION | Description | |:---- |:---- | | `-add` *source* *nameservices* *destination* | Add a mount table entry or update if it exists. | -| `-update` *source* *nameservices* *destination* | Update a mount table entry or create one if it does not exist. | +| `-update` *source* *nameservices* *destination* | Update a mount table entry attribures. | | `-rm` *source* | Remove mount point of specified path. | | `-ls` *path* | List mount points under specified path. | | `-getDestination` *path* | Get the subcluster where a file is or should be created. |