Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import org.apache.hadoop.test.GenericTestUtils;
import org.apache.hadoop.test.LambdaTestUtils;
import org.apache.hadoop.util.ToolRunner;
import org.apache.hadoop.fs.TrashPolicy;
import org.apache.hadoop.ozone.om.TrashPolicyOzone;

import com.google.common.base.Strings;

Expand Down Expand Up @@ -496,6 +498,23 @@ private OzoneConfiguration getClientConfForOFS(
return clientConf;
}

/**
* Helper function to retrieve Ozone client configuration for ozone
* trash testing with TrashPolicyOzone.
* @param hostPrefix Scheme + Authority. e.g. ofs://om-service-test1
* @param configuration Server config to generate client config from.
* @return Config ofs configuration added with fs.trash.classname
* = TrashPolicyOzone.
*/
private OzoneConfiguration getClientConfForOzoneTrashPolicy(
String hostPrefix, OzoneConfiguration configuration) {
OzoneConfiguration clientConf =
getClientConfForOFS(hostPrefix, configuration);
clientConf.setClass("fs.trash.classname", TrashPolicyOzone.class,
TrashPolicy.class);
return clientConf;
}

@Test
public void testDeleteToTrashOrSkipTrash() throws Exception {
final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
Expand Down Expand Up @@ -555,9 +574,89 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
} finally {
shell.close();
fs.close();
}
}

@Test
public void testDeleteTrashNoSkipTrash() throws Exception {

// Test delete from Trash directory removes item from filesystem

// setup configuration to use TrashPolicyOzone
// (default is TrashPolicyDefault)
Copy link
Contributor

@sadanand48 sadanand48 Apr 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neils-dev Please use getClientConfForOFS(hostPrefix,conf) for initialising clientConf here. The integration test is failing because of this

 // Test delete from Trash directory removes item from filesystem

    // setup configuration to use TrashPolicyOzone
    // (default is TrashPolicyDefault)
    final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
    OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix,conf);
    clientConf.setInt(FS_TRASH_INTERVAL_KEY, 60);
    clientConf.setClass("fs.trash.classname", TrashPolicyOzone.class,
                        TrashPolicy.class);
    OzoneFsShell shell = new OzoneFsShell(clientConf);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment @sadanand48. Commit to set configuration in function setting TrashPolicyOzone after getClientConfForOFS as suggested.

Copy link
Contributor

@sadanand48 sadanand48 Apr 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @neils-dev for making the change . Sorry , I wasn't clear . I meant to use the same URI as that of testDeleteToTrashOrSkipTrash i.e to include serviceId as part of the URI. Since this is HA setup ,its required to pass the serviceID. Please use the same URI as that of testDeleteToTrashOrSkipTrash i.e

final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix,conf);

instead of
OzoneConfiguration clientConf = getClientConfForOzoneTrashPolicy("ofs://localhost", conf);

I tried running the individual test in IDE and it's failing. The test only passes when the entire test file is run which is why the integration test passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment @sadanand48 . Commit a change for the service id as suggested. Thanks for following up. BTW, the host set to localhost, as was previously used for the clientconf should not be a problem with the integration test. It was tested both individually hadoop-ozone/integration-test$ mvn -Dtest=TestOzoneShellHA#testDeleteTrashNoSkipTrash test and when the entire suite is run, hadoop-ozone/integration-test$ mvn -Dtest=TestOzoneShellHA test. See attached image.

hadoop-ozone/integration-test$ mvn -Dtest=TestOzoneShellHA#testDeleteTrashNoSkipTrash test
skipTrash

final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
OzoneConfiguration clientConf =
getClientConfForOzoneTrashPolicy(hostPrefix, conf);
OzoneFsShell shell = new OzoneFsShell(clientConf);

int res;

// create volume: vol1 with bucket: bucket1
final String testVolBucket = "/vol1/bucket1";
final String testKey = testVolBucket+"/key1";

final String[] volBucketArgs = new String[] {"-mkdir", "-p", testVolBucket};
final String[] keyArgs = new String[] {"-touch", testKey};
final String[] listArgs = new String[] {"key", "list", testVolBucket};

LOG.info("Executing testDeleteTrashNoSkipTrash: FsShell with args {}",
Arrays.asList(volBucketArgs));
res = ToolRunner.run(shell, volBucketArgs);
Assert.assertEquals(0, res);

// create key: key1 belonging to bucket1
res = ToolRunner.run(shell, keyArgs);
Assert.assertEquals(0, res);

// check directory listing for bucket1 contains 1 key
out.reset();
execute(ozoneShell, listArgs);
Assert.assertEquals(1, getNumOfKeys());

// Test deleting items in trash are discarded (removed from filesystem)
// 1.) remove key1 from bucket1 with fs shell rm command
// 2.) on rm, item is placed in Trash
// 3.) remove Trash directory and all contents,
// check directory listing = 0 items

final String[] rmKeyArgs = new String[] {"-rm", "-R", testKey};
final String[] rmTrashArgs = new String[] {"-rm", "-R",
testVolBucket+"/.Trash"};
final Path trashPathKey1 = Path.mergePaths(new Path(
new OFSPath(testKey).getTrashRoot(), new Path("Current")),
new Path(testKey));
FileSystem fs = FileSystem.get(clientConf);

try {
// on delete key, item is placed in trash
LOG.info("Executing testDeleteTrashNoSkipTrash: FsShell with args {}",
Arrays.asList(rmKeyArgs));
res = ToolRunner.run(shell, rmKeyArgs);
Assert.assertEquals(0, res);

LOG.info("Executing testDeleteTrashNoSkipTrash: key1 deleted moved to"
+" Trash: "+trashPathKey1.toString());
fs.getFileStatus(trashPathKey1);

LOG.info("Executing testDeleteTrashNoSkipTrash: deleting trash FsShell "
+"with args{}: ", Arrays.asList(rmTrashArgs));
res = ToolRunner.run(shell, rmTrashArgs);
Assert.assertEquals(0, res);

out.reset();
// once trash is is removed, trash should be deleted from filesystem
execute(ozoneShell, listArgs);
Assert.assertEquals(0, getNumOfKeys());

} finally {
shell.close();
fs.close();
}

}


@Test
@SuppressWarnings("methodlength")
public void testShQuota() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@
import org.apache.hadoop.fs.FileAlreadyExistsException;
import org.apache.hadoop.fs.permission.FsAction;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.fs.InvalidPathException;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.conf.OMClientConfig;
import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -118,6 +120,28 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}

@Override
public boolean moveToTrash(Path path) throws IOException {
this.fs.getFileStatus(path);
Path trashRoot = this.fs.getTrashRoot(path);

String key = path.toUri().getPath();
LOG.debug("Key path to moveToTrash: "+ key);
String trashRootKey = trashRoot.toUri().getPath();
LOG.debug("TrashrootKey for moveToTrash: "+ trashRootKey);

if (!OzoneFSUtils.isValidName(key)) {
throw new InvalidPathException("Invalid path Name " + key);
}
// first condition tests when length key is <= length trash
// and second when length key > length trash
if ((key.contains(this.fs.TRASH_PREFIX)) && (trashRootKey.startsWith(key))
|| key.startsWith(trashRootKey)) {
return false;
} else {
return super.moveToTrash(path);
}
}

protected class Emptier implements Runnable {

Expand Down