HDFS-5587. add debug information when NFS fails to start with duplicate user or group names. Contributed by Brandon Li

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1548028 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/nfs/nfs3/IdUserGroup.java

@@ -24,6 +24,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
 
@@ -44,13 +45,21 @@ public class IdUserGroup {
   // Do update every 15 minutes
   final static long TIMEOUT = 15 * 60 * 1000; // ms
 
-  // Maps for id to name map. Guarded by this object monitor lock */
+  // Maps for id to name map. Guarded by this object monitor lock
   private BiMap<Integer, String> uidNameMap = HashBiMap.create();
   private BiMap<Integer, String> gidNameMap = HashBiMap.create();
 
   private long lastUpdateTime = 0; // Last time maps were updated
 
-  public IdUserGroup() {
+  static public class DuplicateNameOrIdException extends IOException {
+    private static final long serialVersionUID = 1L;
+
+    public DuplicateNameOrIdException(String msg) {
+      super(msg);
+    }
+  }
+  
+  public IdUserGroup() throws IOException {
     updateMaps();
   }
 
@@ -58,18 +67,34 @@ private boolean isExpired() {
     return lastUpdateTime - System.currentTimeMillis() > TIMEOUT;
   }
 
+  // If can't update the maps, will keep using the old ones
   private void checkAndUpdateMaps() {
     if (isExpired()) {
       LOG.info("Update cache now");
-      updateMaps();
+      try {
+        updateMaps();
+      } catch (IOException e) {
+        LOG.error("Can't update the maps. Will use the old ones,"
+            + " which can potentially cause problem.", e);
+      }
     }
   }
 
+  private static final String DUPLICATE_NAME_ID_DEBUG_INFO = "NFS gateway can't start with duplicate name or id on the host system.\n"
+      + "This is because HDFS (non-kerberos cluster) uses name as the only way to identify a user or group.\n"
+      + "The host system with duplicated user/group name or id might work fine most of the time by itself.\n"
+      + "However when NFS gateway talks to HDFS, HDFS accepts only user and group name.\n"
+      + "Therefore, same name means the same user or same group. To find the duplicated names/ids, one can do:\n"
+      + "<getent passwd | cut -d: -f1,3> and <getent group | cut -d: -f1,3> on Linux systms,\n"
+      + "<dscl . -list /Users UniqueID> and <dscl . -list /Groups PrimaryGroupID> on MacOS.";
+  
   /**
    * Get the whole list of users and groups and save them in the maps.
+   * @throws IOException 
    */
-  private void updateMapInternal(BiMap<Integer, String> map, String name,
-      String command, String regex) throws IOException {
+  @VisibleForTesting
+  public static void updateMapInternal(BiMap<Integer, String> map, String mapName,
+      String command, String regex) throws IOException  {
     BufferedReader br = null;
     try {
       Process process = Runtime.getRuntime().exec(
@@ -79,15 +104,31 @@ private void updateMapInternal(BiMap<Integer, String> map, String name,
       while ((line = br.readLine()) != null) {
         String[] nameId = line.split(regex);
         if ((nameId == null) || (nameId.length != 2)) {
-          throw new IOException("Can't parse " + name + " list entry:" + line);
+          throw new IOException("Can't parse " + mapName + " list entry:" + line);
+        }
+        LOG.debug("add to " + mapName + "map:" + nameId[0] + " id:" + nameId[1]);
+        // HDFS can't differentiate duplicate names with simple authentication
+        Integer key = Integer.valueOf(nameId[1]);
+        String value = nameId[0];
+        if (map.containsKey(key)) {
+          LOG.error(String.format(
+              "Got duplicate id:(%d, %s), existing entry: (%d, %s).\n%s", key,
+              value, key, map.get(key), DUPLICATE_NAME_ID_DEBUG_INFO));
+          throw new DuplicateNameOrIdException("Got duplicate id.");
+        }
+        if (map.containsValue(nameId[0])) {
+          LOG.error(String.format(
+              "Got duplicate name:(%d, %s), existing entry: (%d, %s) \n%s",
+              key, value, map.inverse().get(value), value,
+              DUPLICATE_NAME_ID_DEBUG_INFO));
+          throw new DuplicateNameOrIdException("Got duplicate name");
         }
-        LOG.debug("add " + name + ":" + nameId[0] + " id:" + nameId[1]);
         map.put(Integer.valueOf(nameId[1]), nameId[0]);
       }
-      LOG.info("Updated " + name + " map size:" + map.size());
+      LOG.info("Updated " + mapName + " map size:" + map.size());
       
     } catch (IOException e) {
-      LOG.error("Can't update map " + name);
+      LOG.error("Can't update " + mapName + " map");
       throw e;
     } finally {
       if (br != null) {
@@ -101,24 +142,26 @@ private void updateMapInternal(BiMap<Integer, String> map, String name,
     }
   }
 
-  synchronized public void updateMaps() {
+  synchronized public void updateMaps() throws IOException {
     BiMap<Integer, String> uMap = HashBiMap.create();
     BiMap<Integer, String> gMap = HashBiMap.create();
 
-    try {
-      if (OS.startsWith("Linux")) {
-        updateMapInternal(uMap, "user", LINUX_GET_ALL_USERS_CMD, ":");
-        updateMapInternal(gMap, "group", LINUX_GET_ALL_GROUPS_CMD, ":");
-      } else if (OS.startsWith("Mac")) {
-        updateMapInternal(uMap, "user", MAC_GET_ALL_USERS_CMD, "\\s+");
-        updateMapInternal(gMap, "group", MAC_GET_ALL_GROUPS_CMD, "\\s+");
-      } else {
-        throw new IOException("Platform is not supported:" + OS);
-      }
-    } catch (IOException e) {
-      LOG.error("Can't update maps:" + e);
+    if (!OS.startsWith("Linux") && !OS.startsWith("Mac")) {
+      LOG.error("Platform is not supported:" + OS
+          + ". Can't update user map and group map and"
+          + " 'nobody' will be used for any user and group.");
       return;
     }
+
+    if (OS.startsWith("Linux")) {
+      updateMapInternal(uMap, "user", LINUX_GET_ALL_USERS_CMD, ":");
+      updateMapInternal(gMap, "group", LINUX_GET_ALL_GROUPS_CMD, ":");
+    } else {
+      // Mac
+      updateMapInternal(uMap, "user", MAC_GET_ALL_USERS_CMD, "\\s+");
+      updateMapInternal(gMap, "group", MAC_GET_ALL_GROUPS_CMD, "\\s+");
+    }
+
     uidNameMap = uMap;
     gidNameMap = gMap;
     lastUpdateTime = System.currentTimeMillis();

hadoop-common-project/hadoop-nfs/src/test/java/org/apache/hadoop/nfs/nfs3/TestIdUserGroup.java

@@ -0,0 +1,56 @@
+/**
+ * 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.hadoop.nfs.nfs3;
+
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+
+import org.apache.hadoop.nfs.nfs3.IdUserGroup.DuplicateNameOrIdException;
+import org.junit.Test;
+
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+
+public class TestIdUserGroup {
+
+  @Test
+  public void testDuplicates() throws IOException {
+    String GET_ALL_USERS_CMD = "echo \"root:x:0:0:root:/root:/bin/bash\n"
+        + "hdfs:x:11501:10787:Grid Distributed File System:/home/hdfs:/bin/bash\n"
+        + "hdfs:x:11502:10788:Grid Distributed File System:/home/hdfs:/bin/bash\""
+        + " | cut -d: -f1,3";
+    String GET_ALL_GROUPS_CMD = "echo \"hdfs:*:11501:hrt_hdfs\n"
+        + "mapred:x:497\n" + "mapred2:x:497\"" + " | cut -d: -f1,3";
+    // Maps for id to name map
+    BiMap<Integer, String> uMap = HashBiMap.create();
+    BiMap<Integer, String> gMap = HashBiMap.create();
+
+    try {
+      IdUserGroup.updateMapInternal(uMap, "user", GET_ALL_USERS_CMD, ":");
+      fail("didn't detect the duplicate name");
+    } catch (DuplicateNameOrIdException e) {
+    }
+    
+    try {
+      IdUserGroup.updateMapInternal(gMap, "group", GET_ALL_GROUPS_CMD, ":");
+      fail("didn't detect the duplicate id");
+    } catch (DuplicateNameOrIdException e) {
+    }
+  }
+}

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -777,6 +777,9 @@ Release 2.3.0 - UNRELEASED
 
     HDFS-4997. libhdfs doesn't return correct error codes in most cases (cmccabe)
 
+    HDFS-5587. add debug information when NFS fails to start with duplicate user
+    or group names (brandonli)
+
 Release 2.2.0 - 2013-10-13
 
   INCOMPATIBLE CHANGES