Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: encryption
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The goal is to improve unit test coverage for HDFS trash with encryption zone especially under Kerberos environment. The current unit test TestEncryptionZones#testEncryptionZonewithTrash() has limited coverage on non-Kerberos case.

      1. HDFS-10906.005.patch
        28 kB
        Hanisha Koneru
      2. HDFS-10906.004.patch
        28 kB
        Hanisha Koneru
      3. HDFS-10906.003.patch
        28 kB
        Hanisha Koneru
      4. HDFS-10906.002.patch
        27 kB
        Hanisha Koneru
      5. HDFS-10906.001.patch
        27 kB
        Hanisha Koneru
      6. HDFS-10906.000.patch
        26 kB
        Hanisha Koneru

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10633 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10633/)
        HDFS-10906. Add unit tests for Trash with HDFS encryption zones. (xyao: rev c62ae7107f025091652e79db3edfca5c4dc84e4a)

        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
        • (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestTrashWithSecureEncryptionZones.java
        • (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestTrashWithEncryptionZones.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10633 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10633/ ) HDFS-10906 . Add unit tests for Trash with HDFS encryption zones. (xyao: rev c62ae7107f025091652e79db3edfca5c4dc84e4a) (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestTrashWithSecureEncryptionZones.java (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestTrashWithEncryptionZones.java
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao for reviewing and committing the patch.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao for reviewing and committing the patch.
        Hide
        xyao Xiaoyu Yao added a comment - - edited

        Thanks Hanisha Koneru for the contribution. I've validated the failed Jenkins test org.apache.hadoop.hdfs.server.datanode.TestDirectoryScanner does not repro locally and commit the patch to trunk, branch-2 and branch-2.8.

        Show
        xyao Xiaoyu Yao added a comment - - edited Thanks Hanisha Koneru for the contribution. I've validated the failed Jenkins test org.apache.hadoop.hdfs.server.datanode.TestDirectoryScanner does not repro locally and commit the patch to trunk, branch-2 and branch-2.8.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 18s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 7m 10s trunk passed
        +1 compile 0m 45s trunk passed
        +1 checkstyle 0m 27s trunk passed
        +1 mvnsite 0m 55s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 48s trunk passed
        +1 javadoc 0m 40s trunk passed
        +1 mvninstall 0m 47s the patch passed
        +1 compile 0m 42s the patch passed
        +1 javac 0m 42s the patch passed
        -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 102 unchanged - 1 fixed = 103 total (was 103)
        +1 mvnsite 0m 50s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 48s the patch passed
        +1 javadoc 0m 37s the patch passed
        -1 unit 72m 19s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 19s The patch does not generate ASF License warnings.
        91m 24s



        Reason Tests
        Failed junit tests hadoop.hdfs.server.datanode.TestDirectoryScanner



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10906
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834014/HDFS-10906.005.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux dfaa5de2b0c1 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b733a6f
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17206/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17206/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17206/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17206/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 7m 10s trunk passed +1 compile 0m 45s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 55s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 48s trunk passed +1 javadoc 0m 40s trunk passed +1 mvninstall 0m 47s the patch passed +1 compile 0m 42s the patch passed +1 javac 0m 42s the patch passed -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 102 unchanged - 1 fixed = 103 total (was 103) +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 37s the patch passed -1 unit 72m 19s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 91m 24s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestDirectoryScanner Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10906 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834014/HDFS-10906.005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux dfaa5de2b0c1 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b733a6f Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17206/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/17206/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17206/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17206/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xyao Xiaoyu Yao added a comment -

        Hanisha Koneru, thanks for the update. Patch v4 looks good to me. +1 pending Jenkins.

        Show
        xyao Xiaoyu Yao added a comment - Hanisha Koneru , thanks for the update. Patch v4 looks good to me. +1 pending Jenkins.
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Fixing checkstyle errors in patch v4.
        Unit tests pass locally.

        Show
        hanishakoneru Hanisha Koneru added a comment - Fixing checkstyle errors in patch v4. Unit tests pass locally.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 14s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 8m 8s trunk passed
        +1 compile 0m 47s trunk passed
        +1 checkstyle 0m 28s trunk passed
        +1 mvnsite 0m 56s trunk passed
        +1 mvneclipse 0m 15s trunk passed
        +1 findbugs 1m 53s trunk passed
        +1 javadoc 0m 41s trunk passed
        +1 mvninstall 0m 44s the patch passed
        +1 compile 0m 44s the patch passed
        +1 javac 0m 44s the patch passed
        -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 11 new + 102 unchanged - 1 fixed = 113 total (was 103)
        +1 mvnsite 0m 49s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 56s the patch passed
        +1 javadoc 0m 35s the patch passed
        -1 unit 58m 21s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 22s The patch does not generate ASF License warnings.
        78m 45s



        Reason Tests
        Failed junit tests hadoop.hdfs.server.namenode.web.resources.TestWebHdfsDataLocality
          hadoop.metrics2.sink.TestRollingFileSystemSinkWithHdfs



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10906
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833780/HDFS-10906.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ccc73ef4f436 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / ed9fcbe
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17185/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17185/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17185/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17185/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 8m 8s trunk passed +1 compile 0m 47s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 0m 56s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 53s trunk passed +1 javadoc 0m 41s trunk passed +1 mvninstall 0m 44s the patch passed +1 compile 0m 44s the patch passed +1 javac 0m 44s the patch passed -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 11 new + 102 unchanged - 1 fixed = 113 total (was 103) +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 56s the patch passed +1 javadoc 0m 35s the patch passed -1 unit 58m 21s hadoop-hdfs in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 78m 45s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.web.resources.TestWebHdfsDataLocality   hadoop.metrics2.sink.TestRollingFileSystemSinkWithHdfs Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10906 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833780/HDFS-10906.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ccc73ef4f436 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ed9fcbe Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17185/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/17185/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17185/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17185/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao for reviewing the patch.

        The Path constructor public Path(Path parent, String child) resolves the parent path to only the scheme and authority. The path component is removed. For example,

        Path p1 = new Path("hdfs://localhost:54995/user/hdfs/.Trash/Current”);
        Path p2 = new Path("/zone4/encFile3”);
        Path p = new Path(p1, p2)

        would resolve path p to "hdfs://localhost:55234/zone4/encFile3".
        Thats why I used mergePaths to retain the path component in parent path. The other option would be to remove the leading SEPARATOR ("/") from the child path and then use the Path constructor.

        I have fixed the other issues in patch v3.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao for reviewing the patch. The Path constructor public Path(Path parent, String child) resolves the parent path to only the scheme and authority. The path component is removed. For example, Path p1 = new Path("hdfs://localhost:54995/user/hdfs/.Trash/Current”); Path p2 = new Path("/zone4/encFile3”); Path p = new Path(p1, p2) would resolve path p to "hdfs://localhost:55234/zone4/encFile3". Thats why I used mergePaths to retain the path component in parent path. The other option would be to remove the leading SEPARATOR ("/") from the child path and then use the Path constructor. I have fixed the other issues in patch v3.
        Hide
        xyao Xiaoyu Yao added a comment -

        Thanks for the update Hanisha Koneru. Here are some additional minor issues.

        1. Fix the checkstyle issue
        2. DFSTestUtil#verifyDelete(), since we are dealing with HDFS path, the following
        can be simplified from

        Path trashPath = Path.mergePaths(shell.getCurrentTrashDir(path), path);
        

        to

        Path trashPath = new Path(shell.getCurrentTrashDir(path), path);
        

        TestTrashWithSecureEncryptionZones.java
        3. NIT: Can we put all static imports in a single block before the non-static imports?

        4. The following two counters should be AtomicInteger

        private static int zoneCounter = 1;
        private static int fileCounter = 1;
        
        Show
        xyao Xiaoyu Yao added a comment - Thanks for the update Hanisha Koneru . Here are some additional minor issues. 1. Fix the checkstyle issue 2. DFSTestUtil#verifyDelete(), since we are dealing with HDFS path, the following can be simplified from Path trashPath = Path.mergePaths(shell.getCurrentTrashDir(path), path); to Path trashPath = new Path(shell.getCurrentTrashDir(path), path); TestTrashWithSecureEncryptionZones.java 3. NIT: Can we put all static imports in a single block before the non-static imports? 4. The following two counters should be AtomicInteger private static int zoneCounter = 1; private static int fileCounter = 1;
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 11s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 8m 36s trunk passed
        +1 compile 0m 51s trunk passed
        +1 checkstyle 0m 29s trunk passed
        +1 mvnsite 0m 57s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 50s trunk passed
        +1 javadoc 0m 43s trunk passed
        +1 mvninstall 0m 54s the patch passed
        +1 compile 0m 51s the patch passed
        +1 javac 0m 51s the patch passed
        -0 checkstyle 0m 26s hadoop-hdfs-project/hadoop-hdfs: The patch generated 7 new + 102 unchanged - 1 fixed = 109 total (was 103)
        +1 mvnsite 0m 58s the patch passed
        +1 mvneclipse 0m 11s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 2m 2s the patch passed
        +1 javadoc 0m 38s the patch passed
        +1 unit 66m 42s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        88m 9s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10906
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833229/HDFS-10906.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 1581c8308a65 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 0a85d07
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17144/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17144/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17144/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 8m 36s trunk passed +1 compile 0m 51s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 0m 57s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 50s trunk passed +1 javadoc 0m 43s trunk passed +1 mvninstall 0m 54s the patch passed +1 compile 0m 51s the patch passed +1 javac 0m 51s the patch passed -0 checkstyle 0m 26s hadoop-hdfs-project/hadoop-hdfs: The patch generated 7 new + 102 unchanged - 1 fixed = 109 total (was 103) +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 2s the patch passed +1 javadoc 0m 38s the patch passed +1 unit 66m 42s hadoop-hdfs in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 88m 9s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10906 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833229/HDFS-10906.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1581c8308a65 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0a85d07 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17144/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17144/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17144/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao for reviewing the patch.
        I have addressed the comments in patch v2.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao for reviewing the patch. I have addressed the comments in patch v2.
        Hide
        xyao Xiaoyu Yao added a comment -

        Thanks Hanisha Koneru for working on this to improve the unit test coverage. The patch looks good to me overall. I just have a few comments.

        TestTrashWithEncryptionZones.java
        1. Unused imports
        import org.apache.hadoop.fs.FileStatus;
        import org.junit.rules.Timeout;
        import org.junit.runners.MethodSorters;
        import org.slf4j.Logger;
        import org.slf4j.LoggerFactory;
        import org.junit.AfterClass;
        import org.junit.BeforeClass;
        import org.junit.FixMethodOrder;

        2. We can remove the EncryptionFaultInjector if it is not used in the test.

        @After
        119	  public void teardown() {
        120	    if (cluster != null) {
        121	      cluster.shutdown();
        122	      cluster = null;
        123	    }
        124	    EncryptionFaultInjector.instance = new EncryptionFaultInjector();
        125	...
        

        3. verifyDelete()
        We can use the Path constructor that takes two parameters to avoid concat the
        path with "/"

        203	    final Path trashFile = new Path(shell.getCurrentTrashDir(path) + "/" +
        204	        path);
        

        4. testDeleteEZWithMultipleUsers()
        Some of the verification of positive cases can be simplified by verifyDelete(). Combining with the verifyDelete() in TestTrashWithSecureEncryptionZones, I think we can add it to test utilities such as DFSTestUtil for better reuse.

        NIT: extra empty lines around 156

        TestTrashWithSecureEncryptionZones.java
        5. Unused imports
        import org.slf4j.Logger;
        import org.slf4j.LoggerFactory;

        6. NIT: many instances of local variable len can be a static member of the class

            final int len = 8192; 

        7. testTrashExpunge
        Can we add verification of case where expunge will delete checkpoint inside the
        encryption zone when the whole encryption zone has been deleted and moved to trash
        directory under /user/$USER/.Trash?

        Show
        xyao Xiaoyu Yao added a comment - Thanks Hanisha Koneru for working on this to improve the unit test coverage. The patch looks good to me overall. I just have a few comments. TestTrashWithEncryptionZones.java 1. Unused imports import org.apache.hadoop.fs.FileStatus; import org.junit.rules.Timeout; import org.junit.runners.MethodSorters; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.FixMethodOrder; 2. We can remove the EncryptionFaultInjector if it is not used in the test. @After 119 public void teardown() { 120 if (cluster != null ) { 121 cluster.shutdown(); 122 cluster = null ; 123 } 124 EncryptionFaultInjector.instance = new EncryptionFaultInjector(); 125 ... 3. verifyDelete() We can use the Path constructor that takes two parameters to avoid concat the path with "/" 203 final Path trashFile = new Path(shell.getCurrentTrashDir(path) + "/" + 204 path); 4. testDeleteEZWithMultipleUsers() Some of the verification of positive cases can be simplified by verifyDelete(). Combining with the verifyDelete() in TestTrashWithSecureEncryptionZones, I think we can add it to test utilities such as DFSTestUtil for better reuse. NIT: extra empty lines around 156 TestTrashWithSecureEncryptionZones.java 5. Unused imports import org.slf4j.Logger; import org.slf4j.LoggerFactory; 6. NIT: many instances of local variable len can be a static member of the class final int len = 8192; 7. testTrashExpunge Can we add verification of case where expunge will delete checkpoint inside the encryption zone when the whole encryption zone has been deleted and moved to trash directory under /user/$USER/.Trash?
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        +1 mvninstall 7m 41s trunk passed
        +1 compile 0m 46s trunk passed
        +1 checkstyle 0m 26s trunk passed
        +1 mvnsite 0m 52s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 1m 47s trunk passed
        +1 javadoc 0m 40s trunk passed
        +1 mvninstall 0m 48s the patch passed
        +1 compile 0m 45s the patch passed
        +1 javac 0m 45s the patch passed
        -0 checkstyle 0m 23s hadoop-hdfs-project/hadoop-hdfs: The patch generated 27 new + 0 unchanged - 0 fixed = 27 total (was 0)
        +1 mvnsite 0m 51s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 51s the patch passed
        +1 javadoc 0m 38s the patch passed
        -1 unit 60m 40s hadoop-hdfs in the patch failed.
        +1 asflicense 0m 22s The patch does not generate ASF License warnings.
        80m 23s



        Reason Tests
        Failed junit tests hadoop.hdfs.TestFileAppend
          hadoop.hdfs.server.namenode.TestAddStripedBlockInFBR



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10906
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832717/HDFS-10906.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ceab961b0513 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 809cfd2
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17103/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        unit https://builds.apache.org/job/PreCommit-HDFS-Build/17103/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17103/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17103/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 7m 41s trunk passed +1 compile 0m 46s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 47s trunk passed +1 javadoc 0m 40s trunk passed +1 mvninstall 0m 48s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed -0 checkstyle 0m 23s hadoop-hdfs-project/hadoop-hdfs: The patch generated 27 new + 0 unchanged - 0 fixed = 27 total (was 0) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 51s the patch passed +1 javadoc 0m 38s the patch passed -1 unit 60m 40s hadoop-hdfs in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 80m 23s Reason Tests Failed junit tests hadoop.hdfs.TestFileAppend   hadoop.hdfs.server.namenode.TestAddStripedBlockInFBR Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10906 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832717/HDFS-10906.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ceab961b0513 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 809cfd2 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17103/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/17103/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17103/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17103/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Fixed indentation errors

        Show
        hanishakoneru Hanisha Koneru added a comment - Fixed indentation errors
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 24s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
        +1 mvninstall 9m 30s trunk passed
        +1 compile 1m 0s trunk passed
        +1 checkstyle 0m 32s trunk passed
        +1 mvnsite 1m 3s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 1m 55s trunk passed
        +1 javadoc 0m 47s trunk passed
        +1 mvninstall 0m 56s the patch passed
        +1 compile 0m 51s the patch passed
        +1 javac 0m 51s the patch passed
        -0 checkstyle 0m 36s hadoop-hdfs-project/hadoop-hdfs: The patch generated 598 new + 0 unchanged - 0 fixed = 598 total (was 0)
        +1 mvnsite 1m 0s the patch passed
        +1 mvneclipse 0m 11s the patch passed
        -1 whitespace 0m 0s The patch 468 line(s) with tabs.
        +1 findbugs 2m 2s the patch passed
        +1 javadoc 0m 41s the patch passed
        +1 unit 78m 5s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 25s The patch does not generate ASF License warnings.
        101m 56s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10906
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832592/HDFS-10906.000.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ca54550115a4 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 96b1266
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17095/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/17095/artifact/patchprocess/whitespace-tabs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17095/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17095/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 24s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 9m 30s trunk passed +1 compile 1m 0s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 3s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 55s trunk passed +1 javadoc 0m 47s trunk passed +1 mvninstall 0m 56s the patch passed +1 compile 0m 51s the patch passed +1 javac 0m 51s the patch passed -0 checkstyle 0m 36s hadoop-hdfs-project/hadoop-hdfs: The patch generated 598 new + 0 unchanged - 0 fixed = 598 total (was 0) +1 mvnsite 1m 0s the patch passed +1 mvneclipse 0m 11s the patch passed -1 whitespace 0m 0s The patch 468 line(s) with tabs. +1 findbugs 2m 2s the patch passed +1 javadoc 0m 41s the patch passed +1 unit 78m 5s hadoop-hdfs in the patch passed. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 101m 56s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10906 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832592/HDFS-10906.000.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ca54550115a4 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 96b1266 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/17095/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/17095/artifact/patchprocess/whitespace-tabs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17095/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17095/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        hanishakoneru Hanisha Koneru added a comment -

        Thank you Xiaoyu Yao. I have added two tests to cover the list of test cases.

        Show
        hanishakoneru Hanisha Koneru added a comment - Thank you Xiaoyu Yao . I have added two tests to cover the list of test cases.
        Hide
        xyao Xiaoyu Yao added a comment -

        Agree. Regrading adding Kerberos enabled trash related unit test with KMS/EncryptionZone, we can refactor TestSecureEncryptionZoneWithKMS to share the basic setup for Kerberos (Kerby based MiniKDC) + KMS (MiniKMS) + MiniDFSCluster.

        Show
        xyao Xiaoyu Yao added a comment - Agree. Regrading adding Kerberos enabled trash related unit test with KMS/EncryptionZone, we can refactor TestSecureEncryptionZoneWithKMS to share the basic setup for Kerberos (Kerby based MiniKDC) + KMS (MiniKMS) + MiniDFSCluster.
        Hide
        liuml07 Mingliang Liu added a comment -

        Thanks for the to do list, Xiaoyu Yao. +1 for the proposal. It merits a lot to cover them in unit tests.

        Show
        liuml07 Mingliang Liu added a comment - Thanks for the to do list, Xiaoyu Yao . +1 for the proposal. It merits a lot to cover them in unit tests.
        Hide
        xyao Xiaoyu Yao added a comment - - edited

        Below is a proposed test plan for list of test cases to add:

        1. Trash Checkpoint within the encryption zone
        2. Trash Checkpoint outside the encryption zone when the whole encryption is deleted and moved.
        3. Trash expunge within the encryption zone
        4. Trash expunge outside the encryption zone
        5. Delete file/directory within the encryption zone (Kerberos off)
        6. Delete file/directory within the encryption zone (Kerberos on)
        7. Delete directory with/wo -skipTrash option
        8. Delete empty directory without -skipTrash option
        9. Delete entire encryption zone with different test users
        10. Delete file/directory from Trash within the encryption zone
        11. Namenode restart after Delete file/directory within the encryption zone
        Show
        xyao Xiaoyu Yao added a comment - - edited Below is a proposed test plan for list of test cases to add: Trash Checkpoint within the encryption zone Trash Checkpoint outside the encryption zone when the whole encryption is deleted and moved. Trash expunge within the encryption zone Trash expunge outside the encryption zone Delete file/directory within the encryption zone (Kerberos off) Delete file/directory within the encryption zone (Kerberos on) Delete directory with/wo -skipTrash option Delete empty directory without -skipTrash option Delete entire encryption zone with different test users Delete file/directory from Trash within the encryption zone Namenode restart after Delete file/directory within the encryption zone

          People

          • Assignee:
            hanishakoneru Hanisha Koneru
            Reporter:
            xyao Xiaoyu Yao
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development