You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@twill.apache.org by ch...@apache.org on 2017/04/08 18:08:36 UTC
[13/24] twill git commit: (TWILL-223) Make FileContextLocationFactory
UGI aware
(TWILL-223) Make FileContextLocationFactory UGI aware
- Use different FileContext object based on the caller UGI
- Allows sharing the same factory instance for different user
This closes #47 on GitHub.
Signed-off-by: Terence Yim <ch...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/twill/repo
Commit: http://git-wip-us.apache.org/repos/asf/twill/commit/2c42097c
Tree: http://git-wip-us.apache.org/repos/asf/twill/tree/2c42097c
Diff: http://git-wip-us.apache.org/repos/asf/twill/diff/2c42097c
Branch: refs/heads/site
Commit: 2c42097c5e0de2ef901b96458e0f2745c1d7aeb5
Parents: 4e1cae3
Author: Terence Yim <ch...@apache.org>
Authored: Tue Mar 28 12:53:43 2017 -0700
Committer: Terence Yim <ch...@apache.org>
Committed: Thu Mar 30 01:16:28 2017 -0700
----------------------------------------------------------------------
.../main/java/org/apache/twill/api/Configs.java | 10 +
.../filesystem/FileContextLocationFactory.java | 77 +++-
.../twill/yarn/YarnTwillRunnerService.java | 3 +-
.../filesystem/FileContextLocationTest.java | 58 +--
.../twill/filesystem/LocalLocationTest.java | 23 +-
.../twill/filesystem/LocationTestBase.java | 367 ++++++++++---------
.../java/org/apache/twill/yarn/TwillTester.java | 3 +-
7 files changed, 292 insertions(+), 249 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/twill-api/src/main/java/org/apache/twill/api/Configs.java
----------------------------------------------------------------------
diff --git a/twill-api/src/main/java/org/apache/twill/api/Configs.java b/twill-api/src/main/java/org/apache/twill/api/Configs.java
index 1447a37..9a21489 100644
--- a/twill-api/src/main/java/org/apache/twill/api/Configs.java
+++ b/twill-api/src/main/java/org/apache/twill/api/Configs.java
@@ -83,6 +83,11 @@ public final class Configs {
*/
public static final String LOG_COLLECTION_ENABLED = "twill.log.collection.enabled";
+ /**
+ * The maximum number of FileContext object cached by the FileContextLocationFactory.
+ */
+ public static final String FILE_CONTEXT_CACHE_MAX_SIZE = "twill.file.context.cache.max.size";
+
private Keys() {
}
}
@@ -127,6 +132,11 @@ public final class Configs {
*/
public static final boolean LOG_COLLECTION_ENABLED = true;
+ /**
+ * Default size of the file context cache.
+ */
+ public static final int FILE_CONTEXT_CACHE_MAX_SIZE = 100;
+
private Defaults() {
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocationFactory.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocationFactory.java b/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocationFactory.java
index b8453bc..29a4db5 100644
--- a/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocationFactory.java
+++ b/twill-yarn/src/main/java/org/apache/twill/filesystem/FileContextLocationFactory.java
@@ -18,13 +18,21 @@
package org.apache.twill.filesystem;
import com.google.common.base.Preconditions;
-import com.google.common.base.Throwables;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.UncheckedExecutionException;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
import org.apache.hadoop.fs.FileContext;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.UnsupportedFileSystemException;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.twill.api.Configs;
+import java.io.IOException;
import java.net.URI;
+import java.security.PrivilegedExceptionAction;
import java.util.Objects;
/**
@@ -33,8 +41,8 @@ import java.util.Objects;
public class FileContextLocationFactory implements LocationFactory {
private final Configuration configuration;
- private final FileContext fc;
private final Path pathBase;
+ private final LoadingCache<UserGroupInformation, FileContext> fileContextCache;
/**
* Same as {@link #FileContextLocationFactory(Configuration, String) FileContextLocationFactory(configuration, "/")}.
@@ -49,8 +57,28 @@ public class FileContextLocationFactory implements LocationFactory {
* @param configuration the hadoop configuration
* @param pathBase base path for all non-absolute location created through this {@link LocationFactory}.
*/
- public FileContextLocationFactory(Configuration configuration, String pathBase) {
- this(configuration, createFileContext(configuration), pathBase);
+ public FileContextLocationFactory(final Configuration configuration, String pathBase) {
+ this.configuration = configuration;
+ this.pathBase = new Path(pathBase.startsWith("/") ? pathBase : "/" + pathBase);
+
+ int maxCacheSize = configuration.getInt(Configs.Keys.FILE_CONTEXT_CACHE_MAX_SIZE,
+ Configs.Defaults.FILE_CONTEXT_CACHE_MAX_SIZE);
+ this.fileContextCache = CacheBuilder
+ .newBuilder()
+ .weakKeys()
+ .weakValues()
+ .maximumSize(maxCacheSize)
+ .build(new CacheLoader<UserGroupInformation, FileContext>() {
+ @Override
+ public FileContext load(UserGroupInformation ugi) throws Exception {
+ return ugi.doAs(new PrivilegedExceptionAction<FileContext>() {
+ @Override
+ public FileContext run() throws UnsupportedFileSystemException {
+ return FileContext.getFileContext(configuration);
+ }
+ });
+ }
+ });
}
/**
@@ -59,11 +87,16 @@ public class FileContextLocationFactory implements LocationFactory {
* @param configuration the hadoop configuration
* @param fc {@link FileContext} instance created from the given configuration
* @param pathBase base path for all non-absolute location created through this (@link LocationFactory}.
+ *
+ * @deprecated Use {@link #FileContextLocationFactory(Configuration)}
+ * or {@link #FileContextLocationFactory(Configuration, String)} instead. The {@link FileContext}
+ * provided to this method will only be used if the current user calling any methods of this class
+ * matches with the {@link UserGroupInformation} of the {@link FileContext} instance.
*/
+ @Deprecated
public FileContextLocationFactory(Configuration configuration, FileContext fc, String pathBase) {
- this.configuration = configuration;
- this.fc = fc;
- this.pathBase = new Path(pathBase.startsWith("/") ? pathBase : "/" + pathBase);
+ this(configuration, pathBase);
+ this.fileContextCache.put(fc.getUgi(), fc);
}
@Override
@@ -77,12 +110,14 @@ public class FileContextLocationFactory implements LocationFactory {
} else {
locationPath = new Path(path);
}
+ FileContext fc = getFileContext();
locationPath = locationPath.makeQualified(fc.getDefaultFileSystem().getUri(), pathBase);
return new FileContextLocation(this, fc, locationPath);
}
@Override
public Location create(URI uri) {
+ FileContext fc = getFileContext();
URI contextURI = fc.getWorkingDirectory().toUri();
if (Objects.equals(contextURI.getScheme(), uri.getScheme())
&& Objects.equals(contextURI.getAuthority(), uri.getAuthority())) {
@@ -103,16 +138,32 @@ public class FileContextLocationFactory implements LocationFactory {
@Override
public Location getHomeLocation() {
+ FileContext fc = getFileContext();
// Fix for TWILL-163. FileContext.getHomeDirectory() uses System.getProperty("user.name") instead of UGI
return new FileContextLocation(this, fc,
new Path(fc.getHomeDirectory().getParent(), fc.getUgi().getShortUserName()));
}
/**
- * Returns the {@link FileContext} used by this {@link LocationFactory}.
+ * Returns the {@link FileContext} for the current user based on {@link UserGroupInformation#getCurrentUser()}.
+ *
+ * @throws IllegalStateException if failed to determine the current user or fail to create the FileContext.
+ * @throws RuntimeException if failed to get the {@link FileContext} object for the current user due to exception
*/
public FileContext getFileContext() {
- return fc;
+ try {
+ return fileContextCache.getUnchecked(UserGroupInformation.getCurrentUser());
+ } catch (IOException e) {
+ throw new IllegalStateException("Failed to get current user information", e);
+ } catch (UncheckedExecutionException e) {
+ Throwable cause = e.getCause();
+ if (cause instanceof UnsupportedFileSystemException) {
+ String defaultURI = configuration.get(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY,
+ CommonConfigurationKeysPublic.FS_DEFAULT_NAME_DEFAULT);
+ throw new IllegalStateException("File system with URI '" + defaultURI + "' is not supported", cause);
+ }
+ throw (cause instanceof RuntimeException) ? (RuntimeException) cause : new RuntimeException(cause);
+ }
}
/**
@@ -121,12 +172,4 @@ public class FileContextLocationFactory implements LocationFactory {
public Configuration getConfiguration() {
return configuration;
}
-
- private static FileContext createFileContext(Configuration configuration) {
- try {
- return FileContext.getFileContext(configuration);
- } catch (UnsupportedFileSystemException e) {
- throw Throwables.propagate(e);
- }
- }
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java b/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java
index d2b53b9..a31265e 100644
--- a/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java
+++ b/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillRunnerService.java
@@ -674,7 +674,8 @@ public final class YarnTwillRunnerService implements TwillRunnerService {
private static LocationFactory createDefaultLocationFactory(Configuration configuration) {
try {
FileContext fc = FileContext.getFileContext(configuration);
- return new FileContextLocationFactory(configuration, fc, fc.getHomeDirectory().toUri().getPath());
+ String basePath = fc.getHomeDirectory().toUri().getPath();
+ return new FileContextLocationFactory(configuration, basePath);
} catch (IOException e) {
throw Throwables.propagate(e);
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/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 cbc2781..5629295 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
@@ -17,18 +17,20 @@
*/
package org.apache.twill.filesystem;
-import com.google.common.base.Throwables;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileContext;
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.Assert;
import org.junit.BeforeClass;
+import org.junit.Test;
import java.io.IOException;
-import java.security.PrivilegedAction;
+import java.security.PrivilegedExceptionAction;
/**
*
@@ -36,7 +38,6 @@ import java.security.PrivilegedAction;
public class FileContextLocationTest extends LocationTestBase {
public static MiniDFSCluster dfsCluster;
- private static UserGroupInformation testUGI;
@BeforeClass
public static void init() throws IOException {
@@ -45,9 +46,6 @@ public class FileContextLocationTest extends LocationTestBase {
dfsCluster = new MiniDFSCluster.Builder(conf).numDataNodes(1).build();
// make root world-writable so that we can create all location factories as unprivileged user
dfsCluster.getFileSystem().setPermission(new Path("/"), FsPermission.valueOf("-rwxrwxrwx"));
- // to run these tests not as superuser, make sure to use a user name other than the JVM user
- String userName = System.getProperty("user.name").equals("tester") ? "twiller" : "tester";
- testUGI = UserGroupInformation.createUserForTesting(userName, new String[] { "testgroup" });
}
@AfterClass
@@ -56,39 +54,7 @@ public class FileContextLocationTest extends LocationTestBase {
}
@Override
- protected String getUserName() {
- return testUGI.getUserName();
- }
-
- @Override
- protected String getUserGroup(String ignoredGroupName) {
- return testUGI.getGroupNames()[testUGI.getGroupNames().length - 1];
- }
-
- @Override
protected LocationFactory createLocationFactory(String pathBase) throws Exception {
- return createLocationFactory(pathBase, testUGI);
- }
-
- @Override
- protected LocationFactory createLocationFactory(final String pathBase, UserGroupInformation ugi) throws Exception {
- LocationFactory factory = ugi.doAs(new PrivilegedAction<LocationFactory>() {
- @Override
- public LocationFactory run() {
- try {
- return doCreateLocationFactory(pathBase);
- } catch (IOException e) {
- throw Throwables.propagate(e);
- }
- }
- });
- // make sure the root of the location factory exists and only permits the test user
- Location root = factory.create("/");
- root.mkdirs("rwx------");
- return factory;
- }
-
- protected LocationFactory doCreateLocationFactory(String pathBase) throws IOException {
return new FileContextLocationFactory(dfsCluster.getFileSystem().getConf(), pathBase);
}
@@ -101,4 +67,20 @@ public class FileContextLocationTest extends LocationTestBase {
}
return original;
}
+
+ @Test
+ public void testGetFileContext() throws Exception {
+ final FileContextLocationFactory locationFactory = (FileContextLocationFactory) createLocationFactory("/testGetFC");
+
+ Assert.assertEquals(UserGroupInformation.getCurrentUser(), locationFactory.getFileContext().getUgi());
+
+ UserGroupInformation testUGI = createTestUGI();
+ FileContext fileContext = testUGI.doAs(new PrivilegedExceptionAction<FileContext>() {
+ @Override
+ public FileContext run() throws Exception {
+ return locationFactory.getFileContext();
+ }
+ });
+ Assert.assertEquals(testUGI, fileContext.getUgi());
+ }
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/twill-yarn/src/test/java/org/apache/twill/filesystem/LocalLocationTest.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/test/java/org/apache/twill/filesystem/LocalLocationTest.java b/twill-yarn/src/test/java/org/apache/twill/filesystem/LocalLocationTest.java
index a6b98e8..d48fe8e 100644
--- a/twill-yarn/src/test/java/org/apache/twill/filesystem/LocalLocationTest.java
+++ b/twill-yarn/src/test/java/org/apache/twill/filesystem/LocalLocationTest.java
@@ -18,9 +18,9 @@
package org.apache.twill.filesystem;
import org.apache.hadoop.security.UserGroupInformation;
-import org.junit.Assert;
import java.io.File;
+import java.io.IOException;
/**
*
@@ -28,17 +28,6 @@ import java.io.File;
public class LocalLocationTest extends LocationTestBase {
@Override
- protected String getUserName() {
- return System.getProperty("user.name");
- }
-
- @Override
- protected String getUserGroup(String groupName) {
- String newGroup = System.getProperty("new.group.name");
- return newGroup != null ? newGroup : groupName;
- }
-
- @Override
protected LocationFactory createLocationFactory(String pathBase) throws Exception {
File basePath = new File(tmpFolder.newFolder(), pathBase);
//noinspection ResultOfMethodCallIgnored
@@ -46,13 +35,9 @@ public class LocalLocationTest extends LocationTestBase {
return new LocalLocationFactory(basePath);
}
- protected LocationFactory createLocationFactory(String pathBase, UserGroupInformation ugi) throws Exception {
- return null;
- }
-
@Override
- public void testHomeLocation() throws Exception {
- // For Local location, UGI won't take an effect.
- Assert.assertEquals(System.getProperty("user.name"), createLocationFactory("/").getHomeLocation().getName());
+ protected UserGroupInformation createTestUGI() throws IOException {
+ // In local location, UGI is not supported, hence using the current user as the testing ugi.
+ return UserGroupInformation.getCurrentUser();
}
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/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 3da5f7a..d42cf37 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
@@ -35,6 +35,8 @@ import java.io.OutputStreamWriter;
import java.io.Reader;
import java.io.Writer;
import java.net.URI;
+import java.security.PrivilegedAction;
+import java.security.PrivilegedExceptionAction;
import java.util.List;
/**
@@ -98,16 +100,22 @@ public abstract class LocationTestBase {
@Test
public void testHomeLocation() throws Exception {
- LocationFactory locationFactory = createLocationFactory("/");
+ final LocationFactory locationFactory = createLocationFactory("/");
- // Without UGI, the home location should be the same as the user
- Assert.assertEquals(getUserName(), locationFactory.getHomeLocation().getName());
+ // Without UGI, the home location should be the same as the current user
+ UserGroupInformation currentUser = UserGroupInformation.getCurrentUser();
+ Assert.assertEquals(currentUser.getShortUserName(), locationFactory.getHomeLocation().getName());
// With UGI, the home location should be based on the UGI current user
- UserGroupInformation ugi = UserGroupInformation.createUserForTesting(
- getUserName() + "1", UserGroupInformation.getCurrentUser().getGroupNames());
- locationFactory = createLocationFactory("/", ugi);
- Assert.assertEquals(ugi.getUserName(), locationFactory.getHomeLocation().getName());
+ UserGroupInformation ugi = createTestUGI();
+
+ Location homeLocation = ugi.doAs(new PrivilegedAction<Location>() {
+ @Override
+ public Location run() {
+ return locationFactory.getHomeLocation();
+ }
+ });
+ Assert.assertEquals(ugi.getUserName(), homeLocation.getName());
}
@Test
@@ -184,163 +192,187 @@ public abstract class LocationTestBase {
@Test
public void testOwnerGroup() throws Exception {
- LocationFactory factory = locationFactoryCache.getUnchecked("ownergroup");
- Location location = factory.create("ogtest");
+ final LocationFactory factory = locationFactoryCache.getUnchecked("ownergroup");
+
+ UserGroupInformation testUGI = createTestUGI();
+ Location location = testUGI.doAs(new PrivilegedExceptionAction<Location>() {
+ @Override
+ public Location run() throws Exception {
+ return factory.create("ogtest");
+ }
+ });
+
location.createNew();
- Assert.assertEquals(getUserName(), location.getOwner());
- String newGroup = getUserGroup(location.getGroup());
- location.setGroup(newGroup);
- Assert.assertEquals(newGroup, location.getGroup());
- }
+ Assert.assertEquals(testUGI.getUserName(), location.getOwner());
- @Test
- public void testPermissions() throws IOException {
- LocationFactory factory = locationFactoryCache.getUnchecked("permission1");
-
- // Test permissions setting on createNew calls
- Location location = factory.create("test400-1");
- Assert.assertTrue(location.createNew("400"));
- Assert.assertEquals("r--------", location.getPermissions());
- location = factory.create("test400-2");
- Assert.assertTrue(location.createNew("r--------"));
- Assert.assertEquals("r--------", location.getPermissions());
- Assert.assertFalse(location.createNew("600"));
-
- location = factory.create("test660-1");
- Assert.assertTrue(location.createNew("660"));
- Assert.assertEquals("rw-rw----", location.getPermissions());
- location = factory.create("test660-2");
- Assert.assertTrue(location.createNew("rw-rw----"));
- Assert.assertEquals("rw-rw----", location.getPermissions());
- Assert.assertFalse(location.createNew("600"));
-
- location = factory.create("test644-1");
- Assert.assertTrue(location.createNew("644"));
- Assert.assertEquals("rw-r--r--", location.getPermissions());
- location = factory.create("test644-2");
- Assert.assertTrue(location.createNew("rw-r--r--"));
- Assert.assertEquals("rw-r--r--", location.getPermissions());
- Assert.assertFalse(location.createNew("600"));
-
- // Test permissions setting on getOutputStream calls
- factory = locationFactoryCache.getUnchecked("permission2");
-
- location = factory.create("test400-1");
- location.getOutputStream("400").close();
- Assert.assertEquals("r--------", location.getPermissions());
- location = factory.create("test400-2");
- location.getOutputStream("r--------").close();
- Assert.assertEquals("r--------", location.getPermissions());
-
- location = factory.create("test660-1");
- location.getOutputStream("660").close();
- Assert.assertEquals("rw-rw----", location.getPermissions());
- location = factory.create("test660-2");
- location.getOutputStream("rw-rw----").close();
- Assert.assertEquals("rw-rw----", location.getPermissions());
-
- location = factory.create("test644-1");
- location.getOutputStream("644").close();
- Assert.assertEquals("rw-r--r--", location.getPermissions());
- location = factory.create("test644-2");
- location.getOutputStream("rw-r--r--").close();
- Assert.assertEquals("rw-r--r--", location.getPermissions());
-
- // Test permissions setting on setPermission method
- factory = locationFactoryCache.getUnchecked("permission3");
-
- // Setting permission on non-existed file should have IOException thrown
- location = factory.create("somefile");
- try {
- location.setPermissions("400");
- Assert.fail("IOException expected on setting permission on non-existing Location.");
- } catch (IOException e) {
- // expected
- }
+ String group = testUGI.getGroupNames()[0];
- // Create file with read only permission
- Assert.assertTrue(location.createNew("444"));
- Assert.assertEquals("r--r--r--", location.getPermissions());
- // Change the permission to write only
- location.setPermissions("222");
- Assert.assertEquals("-w--w--w-", location.getPermissions());
+ location.setGroup(group);
+ Assert.assertEquals(group, location.getGroup());
}
@Test
- public void testDirPermissions() throws IOException {
- LocationFactory factory = locationFactoryCache.getUnchecked("permissionD");
+ public void testPermissions() throws IOException, InterruptedException {
+ createTestUGI().doAs(new PrivilegedExceptionAction<Void>() {
+ @Override
+ public Void run() throws Exception {
+ LocationFactory factory = locationFactoryCache.getUnchecked("permission1");
+
+ // Test permissions setting on createNew calls
+ Location location = factory.create("test400-1");
+ Assert.assertTrue(location.createNew("400"));
+ Assert.assertEquals("r--------", location.getPermissions());
+ location = factory.create("test400-2");
+ Assert.assertTrue(location.createNew("r--------"));
+ Assert.assertEquals("r--------", location.getPermissions());
+ Assert.assertFalse(location.createNew("600"));
+
+ location = factory.create("test660-1");
+ Assert.assertTrue(location.createNew("660"));
+ Assert.assertEquals("rw-rw----", location.getPermissions());
+ location = factory.create("test660-2");
+ Assert.assertTrue(location.createNew("rw-rw----"));
+ Assert.assertEquals("rw-rw----", location.getPermissions());
+ Assert.assertFalse(location.createNew("600"));
+
+ location = factory.create("test644-1");
+ Assert.assertTrue(location.createNew("644"));
+ Assert.assertEquals("rw-r--r--", location.getPermissions());
+ location = factory.create("test644-2");
+ Assert.assertTrue(location.createNew("rw-r--r--"));
+ Assert.assertEquals("rw-r--r--", location.getPermissions());
+ Assert.assertFalse(location.createNew("600"));
+
+ // Test permissions setting on getOutputStream calls
+ factory = locationFactoryCache.getUnchecked("permission2");
+
+ location = factory.create("test400-1");
+ location.getOutputStream("400").close();
+ Assert.assertEquals("r--------", location.getPermissions());
+ location = factory.create("test400-2");
+ location.getOutputStream("r--------").close();
+ Assert.assertEquals("r--------", location.getPermissions());
+
+ location = factory.create("test660-1");
+ location.getOutputStream("660").close();
+ Assert.assertEquals("rw-rw----", location.getPermissions());
+ location = factory.create("test660-2");
+ location.getOutputStream("rw-rw----").close();
+ Assert.assertEquals("rw-rw----", location.getPermissions());
+
+ location = factory.create("test644-1");
+ location.getOutputStream("644").close();
+ Assert.assertEquals("rw-r--r--", location.getPermissions());
+ location = factory.create("test644-2");
+ location.getOutputStream("rw-r--r--").close();
+ Assert.assertEquals("rw-r--r--", location.getPermissions());
+
+ // Test permissions setting on setPermission method
+ factory = locationFactoryCache.getUnchecked("permission3");
+
+ // Setting permission on non-existed file should have IOException thrown
+ location = factory.create("somefile");
+ try {
+ location.setPermissions("400");
+ Assert.fail("IOException expected on setting permission on non-existing Location.");
+ } catch (IOException e) {
+ // expected
+ }
+
+ // Create file with read only permission
+ Assert.assertTrue(location.createNew("444"));
+ Assert.assertEquals("r--r--r--", location.getPermissions());
+ // Change the permission to write only
+ location.setPermissions("222");
+ Assert.assertEquals("-w--w--w-", location.getPermissions());
+
+ return null;
+ }
+ });
+ }
- Location location = factory.create("nn");
- String permissions = "rwxr-x---";
- location.mkdirs(permissions);
- Assert.assertTrue(location.exists());
- Assert.assertTrue(location.isDirectory());
- Assert.assertEquals(permissions, location.getPermissions());
-
- permissions = "rwx------";
- location.setPermissions(permissions);
- Assert.assertEquals(permissions, location.getPermissions());
-
- Location child = location.append("p1");
- Location grandchild = child.append("xx");
- permissions = "rwx-w--w-";
- grandchild.mkdirs(permissions);
- Assert.assertTrue(child.isDirectory());
- Assert.assertTrue(grandchild.isDirectory());
- Assert.assertEquals(permissions, child.getPermissions());
- Assert.assertEquals(permissions, grandchild.getPermissions());
-
- permissions = "rwx------";
- child.delete(true);
- Assert.assertFalse(child.exists());
- Location textfile = grandchild.append("a.txt");
- textfile.getOutputStream(permissions).close();
- Assert.assertTrue(child.isDirectory());
- Assert.assertTrue(grandchild.isDirectory());
- Assert.assertFalse(textfile.isDirectory());
- Assert.assertEquals(permissions, child.getPermissions());
- Assert.assertEquals(permissions, grandchild.getPermissions());
- Assert.assertEquals(correctFilePermissions(permissions), textfile.getPermissions());
-
- // mkdirs of existing file
- Location file = factory.create("existingfile");
- Assert.assertTrue(file.createNew("rwx------"));
- Assert.assertFalse(file.mkdirs());
- Assert.assertFalse(file.mkdirs("rwxrwx---"));
-
- // mkdirs where parent is existing file
- file = file.append("newdir");
- Assert.assertFalse(file.mkdirs());
- Assert.assertFalse(file.mkdirs("rwxrwx---"));
-
- // mkdirs of existing directory
- Location dir = factory.create("existingdir");
- Assert.assertTrue(dir.mkdirs());
- Assert.assertFalse(dir.mkdirs());
- Assert.assertFalse(dir.mkdirs("rwxrwx---"));
-
- // mkdirs for existing parent with different permissions -> should not change
- dir.setPermissions("rwx------");
- Assert.assertEquals("rwx------", dir.getPermissions());
- Location newdir = dir.append("newdir");
- Assert.assertTrue(newdir.mkdirs("rwxrwx---"));
- Assert.assertEquals("rwxrwx---", newdir.getPermissions());
- Assert.assertEquals("rwx------", dir.getPermissions());
-
- // mkdirs whithout permission for parent
- Assert.assertTrue(newdir.delete(true));
- dir.setPermissions("r-x------");
- Assert.assertEquals("r-x------", dir.getPermissions());
- try {
- Assert.assertFalse(newdir.mkdirs());
- } catch (AccessControlException e) {
- // expected
- }
- try {
- Assert.assertFalse(newdir.mkdirs("rwxrwx---"));
- } catch (AccessControlException e) {
- // expected
- }
+ @Test
+ public void testDirPermissions() throws IOException, InterruptedException {
+ createTestUGI().doAs(new PrivilegedExceptionAction<Void>() {
+ @Override
+ public Void run() throws Exception {
+
+ LocationFactory factory = locationFactoryCache.getUnchecked("permissionD");
+
+ Location location = factory.create("base");
+ String permissions = "rwxr-x---";
+ location.mkdirs(permissions);
+ Assert.assertTrue(location.exists());
+ Assert.assertTrue(location.isDirectory());
+ Assert.assertEquals(permissions, location.getPermissions());
+
+ permissions = "rwx------";
+ location.setPermissions(permissions);
+ Assert.assertEquals(permissions, location.getPermissions());
+
+ Location child = location.append("child");
+ Location grandchild = child.append("grandchild");
+ permissions = "rwx-w--w-";
+ grandchild.mkdirs(permissions);
+ Assert.assertTrue(child.isDirectory());
+ Assert.assertTrue(grandchild.isDirectory());
+ Assert.assertEquals(permissions, child.getPermissions());
+ Assert.assertEquals(permissions, grandchild.getPermissions());
+
+ permissions = "rwx------";
+ child.delete(true);
+ Assert.assertFalse(child.exists());
+ Location textfile = grandchild.append("a.txt");
+ textfile.getOutputStream(permissions).close();
+ Assert.assertTrue(child.isDirectory());
+ Assert.assertTrue(grandchild.isDirectory());
+ Assert.assertFalse(textfile.isDirectory());
+ Assert.assertEquals(permissions, child.getPermissions());
+ Assert.assertEquals(permissions, grandchild.getPermissions());
+ Assert.assertEquals(correctFilePermissions(permissions), textfile.getPermissions());
+
+ // mkdirs of existing file
+ Location file = factory.create("existingfile");
+ Assert.assertTrue(file.createNew("rwx------"));
+ Assert.assertFalse(file.mkdirs());
+ Assert.assertFalse(file.mkdirs("rwxrwx---"));
+
+ // mkdirs where parent is existing file
+ file = file.append("newdir");
+ Assert.assertFalse(file.mkdirs());
+ Assert.assertFalse(file.mkdirs("rwxrwx---"));
+
+ // mkdirs of existing directory
+ Location dir = factory.create("existingdir");
+ Assert.assertTrue(dir.mkdirs());
+ Assert.assertFalse(dir.mkdirs());
+ Assert.assertFalse(dir.mkdirs("rwxrwx---"));
+
+ // mkdirs for existing parent with different permissions -> should not change
+ dir.setPermissions("rwx------");
+ Assert.assertEquals("rwx------", dir.getPermissions());
+ Location newdir = dir.append("newdir");
+ Assert.assertTrue(newdir.mkdirs("rwxrwx---"));
+ Assert.assertEquals("rwxrwx---", newdir.getPermissions());
+ Assert.assertEquals("rwx------", dir.getPermissions());
+
+ // mkdirs whithout permission for parent
+ Assert.assertTrue(newdir.delete(true));
+ dir.setPermissions("r-x------");
+ Assert.assertEquals("r-x------", dir.getPermissions());
+ try {
+ Assert.assertFalse(newdir.mkdirs());
+ } catch (AccessControlException e) {
+ // expected
+ }
+ try {
+ Assert.assertFalse(newdir.mkdirs("rwxrwx---"));
+ } catch (AccessControlException e) {
+ // expected
+ }
+ return null;
+ }
+ });
}
/**
@@ -349,26 +381,15 @@ public abstract class LocationTestBase {
protected abstract LocationFactory createLocationFactory(String pathBase) throws Exception;
/**
- * Create a location factory rooted at a given path, for the given UGI.
- */
- protected abstract LocationFactory createLocationFactory(String pathBase, UserGroupInformation ugi) throws Exception;
-
- /**
- * Get the user name used for {@link #createLocationFactory(String)}.
- */
- protected abstract String getUserName();
-
- /**
- * Given the group name of a location, return a valid group name to test changing the location's group.
- * 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
}
+
+ protected UserGroupInformation createTestUGI() throws IOException {
+ String userName = System.getProperty("user.name").equals("tester") ? "twiller" : "tester";
+ return UserGroupInformation.createUserForTesting(userName, new String[] { "testgroup" });
+ }
}
http://git-wip-us.apache.org/repos/asf/twill/blob/2c42097c/twill-yarn/src/test/java/org/apache/twill/yarn/TwillTester.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/test/java/org/apache/twill/yarn/TwillTester.java b/twill-yarn/src/test/java/org/apache/twill/yarn/TwillTester.java
index a141176..04902d5 100644
--- a/twill-yarn/src/test/java/org/apache/twill/yarn/TwillTester.java
+++ b/twill-yarn/src/test/java/org/apache/twill/yarn/TwillTester.java
@@ -207,7 +207,8 @@ public class TwillTester extends ExternalResource {
public LocationFactory createLocationFactory() {
try {
FileContext fc = FileContext.getFileContext(config);
- return new FileContextLocationFactory(config, fc, fc.getHomeDirectory().toUri().getPath());
+ String basePath = fc.getHomeDirectory().toUri().getPath();
+ return new FileContextLocationFactory(config, basePath);
} catch (IOException e) {
throw Throwables.propagate(e);
}