You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/04/23 19:06:42 UTC

[GitHub] [hadoop-ozone] smengcl opened a new pull request #865: [WIP] HDDS-2969. Implement ofs://: Add contract test

smengcl opened a new pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865


   ## What changes were proposed in this pull request?
   
   Add OFS contract tests.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-2969
   
   ## How was this patch tested?
   
   By the patch itself.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-632275176


   `TestOMRatisLogParser` failure looks unrelated.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-632859113


   Retest failed to download artifact. Retriggering again.
   ```
   [ERROR] Failed to execute goal on project hadoop-hdds-common: Could not resolve dependencies for project org.apache.hadoop:hadoop-hdds-common:jar:0.6.0-SNAPSHOT: Failed to collect dependencies at org.apache.ratis:ratis-server:jar:0.6.0-490b689-SNAPSHOT: Failed to read artifact descriptor for org.apache.ratis:ratis-server:jar:0.6.0-490b689-SNAPSHOT: Could not transfer artifact org.apache.ratis:ratis-server:pom:0.6.0-490b689-SNAPSHOT from/to apache.snapshots.https (https://repository.apache.org/content/repositories/snapshots): Failed to transfer file https://repository.apache.org/content/repositories/snapshots/org/apache/ratis/ratis-server/0.6.0-490b689-SNAPSHOT/ratis-server-0.6.0-490b689-SNAPSHOT.pom with status code 503 -> [Help 1]
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r419624363



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractGetFileStatus.java
##########
@@ -0,0 +1,66 @@
+/**
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractGetFileStatusTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Ozone contract tests covering getFileStatus.
+ */
+public class ITestRootedOzoneContractGetFileStatus
+    extends AbstractContractGetFileStatusTest {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ITestRootedOzoneContractGetFileStatus.class);
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void teardown() throws Exception {

Review comment:
       Do we need to override teardown and createConfiguration or it is just for the additional LOG?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420378927



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testMkDirDepth1() {

Review comment:
       Because OFS doesn't support deleting volumes (or buckets) at the moment. Even if they are empty.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: [WIP] HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-618615296


   Comments:
   
   1. I tried to reuse the existing contract test files but it Hadoop Common didn't pass parameters to `getTestFileSystem()` or `getTestPath()`. Hence the copy and paste classes.
   
   Questions:
   
   1. RootDir test doesn't seem to pass for the current state of OFS. e.g. rm /* doesn't really work in OFS at the moment:
   <img width="1415" alt="ContractRootDir" src="https://user-images.githubusercontent.com/50227127/80141341-41779f00-855e-11ea-9665-66f3dbd311aa.png">
   
   ~~Maybe I can set the "root" to a bucket to pass the test?~~
   To be fully compliant with RootDir contract test, OFS needs to support rm volumes.
   I think we should learn from RBF which disables these tests: https://github.com/apache/hadoop/blob/ffbb6b6557f4eb8587c7d57cda38f2a0de573f8b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/fs/contract/router/TestRouterHDFSContractRootDirectory.java#L50-L73


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl merged pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl merged pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: [WIP] HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-618615296


   Comments:
   
   1. I tried to reuse the existing contract test files but it Hadoop Common didn't pass parameters to `getTestFileSystem()` or `getTestPath()`. Hence the copy and paste classes.
   
   Questions:
   
   1. RootDir test doesn't seem to pass for the current state of OFS. e.g. rm /* doesn't really work in OFS at the moment:
   <img width="1415" alt="ContractRootDir" src="https://user-images.githubusercontent.com/50227127/80141341-41779f00-855e-11ea-9665-66f3dbd311aa.png">
   
   Maybe I can set the "root" to a bucket to pass the test?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420140845



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {

Review comment:
       It fails because the test tries to delete dirs(volumes) at root:
   ```java
     @Test
     public void testListEmptyRootDirectory() throws IOException {
       skipIfUnsupported(TEST_ROOT_TESTS_ENABLED);
       FileSystem fs = getFileSystem();
       Path root = new Path("/");
       FileStatus[] statuses = fs.listStatus(root);
       for (FileStatus status : statuses) {
         ContractTestUtils.assertDeleted(fs, status.getPath(), true);
       }
   ```
   
   ```java
     public static void assertDeleted(FileSystem fs,
         Path file,
         boolean recursive,
         boolean allowRootOperations) throws IOException {
       rejectRootOperation(file, allowRootOperations);
       assertPathExists(fs, "about to be deleted file", file);
       boolean deleted = fs.delete(file, recursive);
   
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420137590



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractGetFileStatus.java
##########
@@ -0,0 +1,66 @@
+/**
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractGetFileStatusTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Ozone contract tests covering getFileStatus.
+ */
+public class ITestRootedOzoneContractGetFileStatus
+    extends AbstractContractGetFileStatusTest {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ITestRootedOzoneContractGetFileStatus.class);
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void teardown() throws Exception {

Review comment:
       I think it is just for the log. Copied from o3fs contract test.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-637097427


   @xiaoyuyao I believe we can merge this PR now into OFS dev branch.
   As [my dev branch with HDDS-3373 cherry-picked](https://github.com/smengcl/hadoop-ozone/commits/HDDS-2969-with-HDDS-3373-addendum) logs a [successful run](https://github.com/smengcl/hadoop-ozone/runs/721783494). This should prove the test failure in this PR is unrelated.
   
   Will rebase the OFS dev branch after (involves manual changes to OFS classes due to master branch changes in the o3fs classes).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-630318136


   #906 is merged. Rebasing this patch in a moment


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: [WIP] HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-618615296


   Comments:
   
   1. I tried to reuse the existing contract test files but it Hadoop Common didn't pass parameters to `getTestFileSystem()` or `getTestPath()`. Hence the copy and paste classes.
   
   Questions:
   
   1. RootDir test doesn't seem to pass for the current state of OFS. e.g. rm /* doesn't really work in OFS at the moment:
   <img width="1415" alt="ContractRootDir" src="https://user-images.githubusercontent.com/50227127/80141341-41779f00-855e-11ea-9665-66f3dbd311aa.png">
   
   ~~Maybe I can set the "root" to a bucket to pass the test?~~
   To be fully compliant with RootDir contract test, OFS needs to support rm volumes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-624829038


   Update:
   
   Out of the 5 tests I disabled in RootDir, 3 of them are able to pass with my (tentative) recursive delete root(`rm -r /`) and recursive delete volumes support:
   - testRmEmptyRootDirNonRecursive
   - testListEmptyRootDirectory
   - testMkDirDepth1
   
   But still 2 of them won't be able to run because they are trying to create file directly under root:
   - testRmRootRecursive
   - testRmNonEmptyRootDirNonRecursive
   
   I will post the diff here just to run some checks first.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420335990



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testMkDirDepth1() {

Review comment:
       this should be like create and delete an empty volume? why do we fail here?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-629511537


   Opened #928 for HA contract tests.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r419625302



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {

Review comment:
       listEmptyRoot should be OK for OFS?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420143047



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/RootedOzoneContract.java
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+import org.apache.hadoop.hdds.conf.DatanodeRatisServerConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.ratis.RatisHelper;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+
+import org.junit.Assert;
+
+/**
+ * The contract of Rooted Ozone FileSystem (OFS).
+ */
+class RootedOzoneContract extends AbstractFSContract {
+
+  private static MiniOzoneCluster cluster;
+  private static final String CONTRACT_XML = "contract/ozone.xml";
+
+  RootedOzoneContract(Configuration conf) {
+    super(conf);
+    //insert the base features
+    addConfResource(CONTRACT_XML);
+  }
+
+  @Override
+  public String getScheme() {
+    return OzoneConsts.OZONE_OFS_URI_SCHEME;
+  }
+
+  @Override
+  public Path getTestPath() {
+    return new Path("/testvol1/testbucket1/test");
+  }
+
+  public static void createCluster() throws IOException {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_SERVER_PREFIX_KEY + "." +
+                    DatanodeRatisServerConfig.RATIS_SERVER_REQUEST_TIMEOUT_KEY,
+            3, TimeUnit.SECONDS);
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_SERVER_PREFIX_KEY + "." +
+                    DatanodeRatisServerConfig.
+                            RATIS_SERVER_WATCH_REQUEST_TIMEOUT_KEY,
+            10, TimeUnit.SECONDS);
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_CLIENT_PREFIX_KEY + "." +
+                    "rpc.request.timeout",
+            3, TimeUnit.SECONDS);
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_CLIENT_PREFIX_KEY + "." +
+                    "watch.request.timeout",
+            10, TimeUnit.SECONDS);
+    conf.addResource(CONTRACT_XML);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(5).build();

Review comment:
       I think we need another set of classes to do this but initialize with `MiniOzoneHAClusterImpl` instead.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-637097427


   @xiaoyuyao I believe we can merge this PR now into OFS dev branch.
   As [my dev branch with HDDS-3373 cherry-picked](https://github.com/smengcl/hadoop-ozone/commits/HDDS-2969-with-HDDS-3373-addendum) logs a [successful run](https://github.com/smengcl/hadoop-ozone/runs/721783494).
   
   Will rebase the OFS dev branch after.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-627515342


   @xiaoyuyao 
   The volume and bucket deletion support work will be done in HDDS-3494 (https://github.com/apache/hadoop-ozone/pull/906).
   
   I will remove the commits in this PR that tentatively support volume and bucket deletion.
   
   ~I believe we can push the contract tests without volume and bucket deletion support at the moment. When HDDS-3494 is committed, I can re-enable those disabled RootDir contract tests.~
   After an offline discussion with @xiaoyuyao we think it's better to merge HDDS-3494 before this one.
   
   Marking this one blocked by #904 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-632967135


   > @smengcl Please rebase the branch to pick up this change from master #867. That should fix the CI issue.
   > [HDDS-3373](https://issues.apache.org/jira/browse/HDDS-3373). Intermittent failure in TestOMRatisLogParser (#867)
   > 
   > This seems to be an addendum commit in addition to the one that is currently in [HDDS-2665](https://issues.apache.org/jira/browse/HDDS-2665)-ofs branch.
   > [HDDS-3373](https://issues.apache.org/jira/browse/HDDS-3373). Intermittent failure in TestDnRatisLogParser and TestOMRatisLogParser (#858)
   
   I attempted to rebase the OFS branch a few day ago locally but unfortunately it won't compile, implying changes to OFS classes is required due to other master branch work. I will do the rebase in another jira.
   
   In the mean time I have picked up the addendum patch for HDDS-3373 in another dev branch just to run the tests:
   https://github.com/smengcl/hadoop-ozone/commits/HDDS-2969-with-HDDS-3373-addendum


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420141875



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testMkDirDepth1() {

Review comment:
       It failed during clean up (assertDeleted):
   ```java
     @Test
     public void testMkDirDepth1() throws Throwable {
       FileSystem fs = getFileSystem();
       Path dir = new Path("/testmkdirdepth1");
       assertPathDoesNotExist("directory already exists", dir);
       fs.mkdirs(dir);
       assertIsDirectory(dir);
       assertPathExists("directory already exists", dir);
       assertDeleted(dir, true);
     }
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: [WIP] HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-618615296


   Comments:
   
   1. I tried to reuse the existing contract test files but it Hadoop Common didn't pass parameters to `getTestFileSystem()` or `getTestPath()`. Hence the copy and paste classes.
   
   Questions:
   
   1. RootDir test doesn't seem to pass for the current state of OFS. e.g. rm /* doesn't really work in OFS at the moment:
   <img width="1415" alt="ContractRootDir" src="https://user-images.githubusercontent.com/50227127/80141341-41779f00-855e-11ea-9665-66f3dbd311aa.png">
   
   ~~Maybe I can set the "root" to a bucket to pass the test?~~
   To be fully compliant with RootDir contract test, OFS needs to support deleting volumes with `rm` command.
   
   I think we should learn from RBF which [disables some RootDir tests](https://github.com/apache/hadoop/blob/ffbb6b6557f4eb8587c7d57cda38f2a0de573f8b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/fs/contract/router/TestRouterHDFSContractRootDirectory.java#L50-L73).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420137590



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractGetFileStatus.java
##########
@@ -0,0 +1,66 @@
+/**
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractGetFileStatusTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Ozone contract tests covering getFileStatus.
+ */
+public class ITestRootedOzoneContractGetFileStatus
+    extends AbstractContractGetFileStatusTest {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ITestRootedOzoneContractGetFileStatus.class);
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void teardown() throws Exception {

Review comment:
       Copied from o3fs contract test. I believe it is just for the log.
   
   Will remove this.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420378374



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {

Review comment:
       Yes. I see `testvol1`, which is specified in `RootedOzoneContract#getTestPath`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420140845



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {

Review comment:
       It fails because the test tries to delete dirs(volumes) at root:
   ```java
     @Test
     public void testListEmptyRootDirectory() throws IOException {
       skipIfUnsupported(TEST_ROOT_TESTS_ENABLED);
       FileSystem fs = getFileSystem();
       Path root = new Path("/");
       FileStatus[] statuses = fs.listStatus(root);
       for (FileStatus status : statuses) {
         ContractTestUtils.assertDeleted(fs, status.getPath(), true);
       }
   ```
   then:
   ```java
     public static void assertDeleted(FileSystem fs,
         Path file,
         boolean recursive,
         boolean allowRootOperations) throws IOException {
       rejectRootOperation(file, allowRootOperations);
       assertPathExists(fs, "about to be deleted file", file);
       boolean deleted = fs.delete(file, recursive);
   
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-627515342


   @xiaoyuyao 
   The volume and bucket deletion support work will be done in HDDS-3494 (https://github.com/apache/hadoop-ozone/pull/906).
   
   I will remove the commits in this PR that tentatively support volume and bucket deletion.
   
   ~I believe we can push the contract tests without volume and bucket deletion support at the moment. When HDDS-3494 is committed, I can re-enable those disabled RootDir contract tests.~
   After an offline discussion with @xiaoyuyao we think it's better to merge HDDS-3494 before this one.
   
   Marking this one blocked by #906 .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl edited a comment on pull request #865: [WIP] HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-618615296


   Comments:
   
   1. I tried to reuse the existing contract test files but it Hadoop Common didn't pass parameters to `getTestFileSystem()` or `getTestPath()`. Hence the copy and paste classes.
   
   Questions:
   
   1. RootDir test doesn't seem to pass for the current state of OFS. e.g. rm /* doesn't really work in OFS at the moment:
   <img width="1415" alt="ContractRootDir" src="https://user-images.githubusercontent.com/50227127/80141341-41779f00-855e-11ea-9665-66f3dbd311aa.png">
   
   ~~Maybe I can set the "root" to a bucket to pass the test?~~
   To be fully compliant with RootDir contract test, OFS needs to support deleting volumes with `rm` command.
   
   I think we should learn from RBF which disables these tests: https://github.com/apache/hadoop/blob/ffbb6b6557f4eb8587c7d57cda38f2a0de573f8b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/fs/contract/router/TestRouterHDFSContractRootDirectory.java#L50-L73


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r419625577



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testMkDirDepth1() {

Review comment:
       mkdirDepth1 should be OK for admin? Can we add some configuration to allow this?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r420335487



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/ITestRootedOzoneContractRootDir.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.contract.AbstractContractRootDirectoryTest;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+
+/**
+ * Ozone contract test for ROOT directory operations.
+ */
+public class ITestRootedOzoneContractRootDir extends
+    AbstractContractRootDirectoryTest {
+
+  @BeforeClass
+  public static void createCluster() throws IOException {
+    RootedOzoneContract.createCluster();
+  }
+
+  @AfterClass
+  public static void teardownCluster() throws IOException {
+    RootedOzoneContract.destroyCluster();
+  }
+
+  @Override
+  protected AbstractFSContract createContract(Configuration conf) {
+    return new RootedOzoneContract(conf);
+  }
+
+  @Override
+  public void testRmEmptyRootDirNonRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testRmRootRecursive() {
+    // OFS doesn't support deleting volumes via rm
+  }
+
+  @Override
+  public void testListEmptyRootDirectory() {

Review comment:
       are you seeing the root is not empty even though the test claim to list empty root dir?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-632902658


   @smengcl Please rebase the branch to pick up this change from master #867. That should fix the CI issue. 
   HDDS-3373. Intermittent failure in TestOMRatisLogParser (#867)
   
   This seems to be an addendum commit in addition to the one that is currently in HDDS-2665-ofs branch. 
   HDDS-3373. Intermittent failure in TestDnRatisLogParser and TestOMRatisLogParser (#858)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#discussion_r419628616



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/rooted/RootedOzoneContract.java
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.fs.ozone.contract.rooted;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.AbstractFSContract;
+import org.apache.hadoop.hdds.conf.DatanodeRatisServerConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.ratis.RatisHelper;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+
+import org.junit.Assert;
+
+/**
+ * The contract of Rooted Ozone FileSystem (OFS).
+ */
+class RootedOzoneContract extends AbstractFSContract {
+
+  private static MiniOzoneCluster cluster;
+  private static final String CONTRACT_XML = "contract/ozone.xml";
+
+  RootedOzoneContract(Configuration conf) {
+    super(conf);
+    //insert the base features
+    addConfResource(CONTRACT_XML);
+  }
+
+  @Override
+  public String getScheme() {
+    return OzoneConsts.OZONE_OFS_URI_SCHEME;
+  }
+
+  @Override
+  public Path getTestPath() {
+    return new Path("/testvol1/testbucket1/test");
+  }
+
+  public static void createCluster() throws IOException {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_SERVER_PREFIX_KEY + "." +
+                    DatanodeRatisServerConfig.RATIS_SERVER_REQUEST_TIMEOUT_KEY,
+            3, TimeUnit.SECONDS);
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_SERVER_PREFIX_KEY + "." +
+                    DatanodeRatisServerConfig.
+                            RATIS_SERVER_WATCH_REQUEST_TIMEOUT_KEY,
+            10, TimeUnit.SECONDS);
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_CLIENT_PREFIX_KEY + "." +
+                    "rpc.request.timeout",
+            3, TimeUnit.SECONDS);
+    conf.setTimeDuration(
+            RatisHelper.HDDS_DATANODE_RATIS_CLIENT_PREFIX_KEY + "." +
+                    "watch.request.timeout",
+            10, TimeUnit.SECONDS);
+    conf.addResource(CONTRACT_XML);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(5).build();

Review comment:
       Is it possible to add contract test in the context of HA setup as well as the non-HA?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #865: HDDS-2969. Implement ofs://: Add contract test

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #865:
URL: https://github.com/apache/hadoop-ozone/pull/865#issuecomment-632373491


   Seems HDDS-3373 still not fix the issue completely on TestOMRatisLogParser. Agree this does not seem to relate to the change here. I will trigger another CI just in case. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org