You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@twill.apache.org by an...@apache.org on 2017/01/27 22:11:33 UTC
twill git commit: fix permission test for Hadoop 2.0
Repository: twill
Updated Branches:
refs/heads/master fb261fa76 -> 2f1032100
fix permission test for Hadoop 2.0
This closes #27 from GitHub.
Signed-off-by: anew <an...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/twill/repo
Commit: http://git-wip-us.apache.org/repos/asf/twill/commit/2f103210
Tree: http://git-wip-us.apache.org/repos/asf/twill/tree/2f103210
Diff: http://git-wip-us.apache.org/repos/asf/twill/diff/2f103210
Branch: refs/heads/master
Commit: 2f1032100223cbf2a10c686e0ec69f06736fdeb8
Parents: fb261fa
Author: anew <an...@cask.co>
Authored: Thu Jan 26 19:54:21 2017 -0800
Committer: anew <an...@apache.org>
Committed: Fri Jan 27 14:10:48 2017 -0800
----------------------------------------------------------------------
.../twill/filesystem/FileContextLocation.java | 21 +++++++++++++++-----
.../apache/twill/filesystem/HDFSLocation.java | 20 ++++++++++++++-----
.../filesystem/FileContextLocationTest.java | 11 ++++++++++
.../twill/filesystem/LocationTestBase.java | 10 +++++++++-
4 files changed, 51 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/twill/blob/2f103210/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocation.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocation.java b/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocation.java
index c50a301..8ccd180 100644
--- a/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocation.java
+++ b/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocation.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdfs.HAUtil;
+import org.apache.hadoop.security.AccessControlException;
import java.io.FileNotFoundException;
import java.io.IOException;
@@ -199,13 +200,16 @@ final class FileContextLocation implements Location {
@Override
public boolean mkdirs() throws IOException {
- if (fc.util().exists(path)) {
- return false;
- }
try {
+ if (fc.util().exists(path)) {
+ return false;
+ }
fc.mkdir(path, null, true);
return true;
- } catch (FileAlreadyExistsException e) {
+ } catch (FileAlreadyExistsException | AccessControlException e) {
+ // curiously, if one of the parent dirs exists but is a file, Hadoop throws this:
+ // org.apache...AccessControlException: Permission denied: user=..., access=EXECUTE, inode=".../existingfile"
+ // however, if the directory itself exists, it will throw FileAlreadyExistsException
return false;
}
}
@@ -220,9 +224,16 @@ final class FileContextLocation implements Location {
* We cannot use the fs.mkdirs() because that would apply the umask to the permissions.
*/
private boolean mkdirs(Path path, FsPermission permission) throws IOException {
- if (fc.util().exists(path)) {
+ try {
+ if (fc.util().exists(path)) {
+ return false;
+ }
+ } catch (AccessControlException e) {
+ // curiously, if one of the parent dirs exists but is a file, Hadoop throws this:
+ // org.apache...AccessControlException: Permission denied: user=..., access=EXECUTE, inode=".../existingfile"
return false;
}
+
Path parent = path.getParent();
if (null == parent) {
return false;
http://git-wip-us.apache.org/repos/asf/twill/blob/2f103210/twill-yarn/src/main/java/org/apache/twill/filesystem/HDFSLocation.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/filesystem/HDFSLocation.java b/twill-yarn/src/main/java/org/apache/twill/filesystem/HDFSLocation.java
index 43b37da..ef1138e 100644
--- a/twill-yarn/src/main/java/org/apache/twill/filesystem/HDFSLocation.java
+++ b/twill-yarn/src/main/java/org/apache/twill/filesystem/HDFSLocation.java
@@ -27,6 +27,7 @@ import org.apache.hadoop.fs.Options;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdfs.DistributedFileSystem;
+import org.apache.hadoop.security.AccessControlException;
import java.io.IOException;
import java.io.InputStream;
@@ -223,12 +224,15 @@ final class HDFSLocation implements Location {
@Override
public boolean mkdirs() throws IOException {
- if (fs.exists(path)) {
- return false;
- }
try {
+ if (fs.exists(path)) {
+ return false;
+ }
return fs.mkdirs(path);
- } catch (FileAlreadyExistsException e) {
+ } catch (FileAlreadyExistsException | AccessControlException e) {
+ // curiously, if one of the parent dirs exists but is a file, Hadoop throws this:
+ // org.apache...AccessControlException: Permission denied: user=..., access=EXECUTE, inode=".../existingfile"
+ // however, if the directory itself exists, it will throw FileAlreadyExistsException
return false;
}
}
@@ -243,7 +247,13 @@ final class HDFSLocation implements Location {
* We cannot use the fs.mkdirs() because that would apply the umask to the permissions.
*/
private boolean mkdirs(Path path, FsPermission permission) throws IOException {
- if (fs.exists(path)) {
+ try {
+ if (fs.exists(path)) {
+ return false;
+ }
+ } catch (AccessControlException e) {
+ // curiously, if one of the parent dirs exists but is a file, Hadoop throws this:
+ // org.apache...AccessControlException: Permission denied: user=..., access=EXECUTE, inode=".../existingfile"
return false;
}
Path parent = path.getParent();
http://git-wip-us.apache.org/repos/asf/twill/blob/2f103210/twill-yarn/src/test/java/org/apache/twill/filesystem/FileContextLocationTest.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/test/java/org/apache/twill/filesystem/FileContextLocationTest.java b/twill-yarn/src/test/java/org/apache/twill/filesystem/FileContextLocationTest.java
index 4ed59a7..cbc2781 100644
--- a/twill-yarn/src/test/java/org/apache/twill/filesystem/FileContextLocationTest.java
+++ b/twill-yarn/src/test/java/org/apache/twill/filesystem/FileContextLocationTest.java
@@ -23,6 +23,7 @@ import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.twill.internal.yarn.YarnUtils;
import org.junit.AfterClass;
import org.junit.BeforeClass;
@@ -90,4 +91,14 @@ public class FileContextLocationTest extends LocationTestBase {
protected LocationFactory doCreateLocationFactory(String pathBase) throws IOException {
return new FileContextLocationFactory(dfsCluster.getFileSystem().getConf(), pathBase);
}
+
+ @Override
+ protected String correctFilePermissions(String original) {
+ if (YarnUtils.HadoopVersions.HADOOP_20.equals(YarnUtils.getHadoopVersion())) {
+ return original.substring(0, 2) + '-' + // strip the x for owner
+ original.substring(3, 5) + '-' + // strip the x for group
+ original.substring(6, 8) + '-'; // strip the x for world;
+ }
+ return original;
+ }
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2f103210/twill-yarn/src/test/java/org/apache/twill/filesystem/LocationTestBase.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/test/java/org/apache/twill/filesystem/LocationTestBase.java b/twill-yarn/src/test/java/org/apache/twill/filesystem/LocationTestBase.java
index 5931ada..3da5f7a 100644
--- a/twill-yarn/src/test/java/org/apache/twill/filesystem/LocationTestBase.java
+++ b/twill-yarn/src/test/java/org/apache/twill/filesystem/LocationTestBase.java
@@ -300,7 +300,7 @@ public abstract class LocationTestBase {
Assert.assertFalse(textfile.isDirectory());
Assert.assertEquals(permissions, child.getPermissions());
Assert.assertEquals(permissions, grandchild.getPermissions());
- Assert.assertEquals(permissions, textfile.getPermissions());
+ Assert.assertEquals(correctFilePermissions(permissions), textfile.getPermissions());
// mkdirs of existing file
Location file = factory.create("existingfile");
@@ -363,4 +363,12 @@ public abstract class LocationTestBase {
* If no suitable group name is known, the passed-in group name can be returned.
*/
protected abstract String getUserGroup(String groupName);
+
+ /**
+ * Some older versions of Hadoop always strip the execute permission from files (but keep it for directories).
+ * This allows subclasses to correct the expected file permissions, based on the Hadoop version (if any).
+ */
+ protected String correctFilePermissions(String expectedFilePermissions) {
+ return expectedFilePermissions; // unchanged by default
+ }
}