This patch applies to trunk. Once reviewed I will post the 1.0 patch

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12541046/HADOOP-8611.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

org.apache.hadoop.ha.TestZKFailoverController

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1304//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1304//console

This message is automatically generated.

core-tests failures were time out issues, verified locally on trunk and my branch that TestZKFailoverController tests pass.

TestZKFailoverController timing out is a known issue, see HADOOP-8591.

The code looks good, but I have a few comments.

1. There are tabs in the patch, please update it to use spaces instead.
2. The way the patch is getting the impl instance is very repetative. I think it would be simpler to do the following

   Class<GroupMappingServiceProvider> clazz = conf.getClass(CommonConfigurationKeys.HADOOP_SECURITY_GROUP_MAPPING, 
                                    ShellBasedUnixGroupsMapping.class, 
                                    GroupMappingServiceProvider.class);
   if (conf.getBoolean(CommonConfigurationKeys.HADOOP_SECURITY_GROUP_MAPPING_ALLOW_FALLBACK, false) &&
     !NativeCodeLoader.isNativeCodeLoaded()) {
     LOG.info("Falling back to Shell Based Groups");
     clazz = ShellBasedUnixGroupsMapping.class;
   }
   impl = ReflectionUtils.newInstance(clazz, conf);
   

3. It would be good to update ./hadoop-hdfs-project/hadoop-hdfs/src/main/docs/src/documentation/content/xdocs/hdfs_permissions_guide.xml with the new config option
4. Do we want to check explicitly for JniBasedUnixGroupsMapping and JniBasedUnixGroupsNetgroupMapping? or perhaps move some of this code over into those classes explicitly instead? It seems like configuring LdapGroupsMapping with fallback enabled and non-native code would never work. Also there would be issues with JniBasedUnixGroupsNetgroupMapping and fallback. Is this the reason for having the fallback enable?

Uploaded new patch, isolated the fallback functionality to 2 new specific classes which can be specified within the current configuration scheme.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12541230/HADOOP-8611.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

org.apache.hadoop.ha.TestZKFailoverController

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1319//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1319//console

This message is automatically generated.

Patch for branch-1.0.2

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12541277/HADOOP-8611-branch1.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in .

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1321//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1321//console

This message is automatically generated.

I have two comments.

1. if (LOG.isDebugEnabled())
         LOG.debug("Group mapping impl=" + impl.getClass().getName());
   

   needs curly braces around the if body.

2. The 1.0 branch refers to running mvn test, but 1.0 does not support mvn. Please either update them or remove them.

removed mvn references

added curly braces

added a new patch for branch 1 to remove mvn references and added curly braces to if statement
added a new patch for trunk to add curly braces to if statement

+1 the changes look good I'll check it in.

It might be good to consider filing a separate JIRA to make the default groups implementation the JNI shell with fallback.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12541645/HADOOP-8611.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1330//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1330//console

This message is automatically generated.

Thanks Rob,

I checked this into trunk, branch-2, branch-2.1.0-alpha and branch-0.23.

Integrated in Hadoop-Common-trunk-Commit #2607 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2607/)
HADOOP-8611. Allow fall-back to the shell-based implementation when JNI-based users-group mapping fails (Robert Parker via bobby) (Revision 1375221)

Result = SUCCESS
bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1375221
Files :

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupFallback.java

Integrated in Hadoop-Hdfs-trunk-Commit #2671 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2671/)
HADOOP-8611. Allow fall-back to the shell-based implementation when JNI-based users-group mapping fails (Robert Parker via bobby) (Revision 1375221)

Result = SUCCESS
bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1375221
Files :

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupFallback.java

Integrated in Hadoop-Mapreduce-trunk-Commit #2636 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2636/)
HADOOP-8611. Allow fall-back to the shell-based implementation when JNI-based users-group mapping fails (Robert Parker via bobby) (Revision 1375221)

Result = FAILURE
bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1375221
Files :

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupFallback.java

Integrated in Hadoop-Hdfs-0.23-Build #350 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/350/)
svn merge -c 1375221 FIXES: HADOOP-8611. Allow fall-back to the shell-based implementation when JNI-based users-group mapping fails (Robert Parker via bobby) (Revision 1375224)

Result = SUCCESS
bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1375224
Files :

• /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java
• /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java
• /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupFallback.java

Integrated in Hadoop-Hdfs-trunk #1141 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1141/)
HADOOP-8611. Allow fall-back to the shell-based implementation when JNI-based users-group mapping fails (Robert Parker via bobby) (Revision 1375221)

Result = FAILURE
bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1375221
Files :

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupFallback.java

Integrated in Hadoop-Mapreduce-trunk #1173 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1173/)
HADOOP-8611. Allow fall-back to the shell-based implementation when JNI-based users-group mapping fails (Robert Parker via bobby) (Revision 1375221)

Result = FAILURE
bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1375221
Files :

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupFallback.java

added 1.2.0 to fixVersion, per CHANGES.txt and above comments.