Browse Source

HDDS-568. ozone sh volume info, update, delete operations fail when volume name is not prefixed by /.
Contributed by Dinesh Chitlangia.

Anu Engineer 6 years ago
parent
commit
4de2dc2699

+ 48 - 0
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestOzoneShell.java

@@ -283,6 +283,33 @@ public class TestOzoneShell {
       GenericTestUtils.assertExceptionContains(
           "Info Volume failed, error:VOLUME_NOT_FOUND", e);
     }
+
+
+    volumeName = "volume" + RandomStringUtils.randomNumeric(5);
+    volumeArgs = VolumeArgs.newBuilder()
+        .setOwner("bilbo")
+        .setQuota("100TB")
+        .build();
+    client.createVolume(volumeName, volumeArgs);
+    volume = client.getVolumeDetails(volumeName);
+    assertNotNull(volume);
+
+    //volumeName prefixed with /
+    String volumeNameWithSlashPrefix = "/" + volumeName;
+    args = new String[] {"volume", "delete",
+        url + "/" + volumeNameWithSlashPrefix};
+    execute(shell, args);
+    output = out.toString();
+    assertTrue(output.contains("Volume " + volumeName + " is deleted"));
+
+    // verify if volume has been deleted
+    try {
+      client.getVolumeDetails(volumeName);
+      fail("Get volume call should have thrown.");
+    } catch (IOException e) {
+      GenericTestUtils.assertExceptionContains(
+          "Info Volume failed, error:VOLUME_NOT_FOUND", e);
+    }
   }
 
   @Test
@@ -295,6 +322,7 @@ public class TestOzoneShell {
         .build();
     client.createVolume(volumeName, volumeArgs);
 
+    //volumeName supplied as-is
     String[] args = new String[] {"volume", "info", url + "/" + volumeName};
     execute(shell, args);
 
@@ -303,6 +331,17 @@ public class TestOzoneShell {
     assertTrue(output.contains("createdOn")
         && output.contains(OzoneConsts.OZONE_TIME_ZONE));
 
+    //volumeName prefixed with /
+    String volumeNameWithSlashPrefix = "/" + volumeName;
+    args = new String[] {"volume", "info",
+        url + "/" + volumeNameWithSlashPrefix};
+    execute(shell, args);
+
+    output = out.toString();
+    assertTrue(output.contains(volumeName));
+    assertTrue(output.contains("createdOn")
+        && output.contains(OzoneConsts.OZONE_TIME_ZONE));
+
     // test infoVolume with invalid volume name
     args = new String[] {"volume", "info",
         url + "/" + volumeName + "/invalid-name"};
@@ -365,6 +404,15 @@ public class TestOzoneShell {
     vol = client.getVolumeDetails(volumeName);
     assertEquals(newUser, vol.getOwner());
 
+    //volume with / prefix
+    String volumeWithPrefix = "/" + volumeName;
+    String newUser2 = "new-user2";
+    args = new String[] {"volume", "update", url + "/" + volumeWithPrefix,
+        "--user", newUser2};
+    execute(shell, args);
+    vol = client.getVolumeDetails(volumeName);
+    assertEquals(newUser2, vol.getOwner());
+
     // test error conditions
     args = new String[] {"volume", "update", url + "/invalid-volume",
         "--user", newUser};

+ 28 - 0
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/Handler.java

@@ -21,6 +21,8 @@ package org.apache.hadoop.ozone.web.ozShell;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.concurrent.Callable;
 
 import org.apache.hadoop.conf.Configuration;
@@ -153,6 +155,32 @@ public abstract class Handler implements Callable<Void> {
     }
   }
 
+  /**
+   *
+   * @param uri
+   * @return volumeName
+   * @throws Exception
+   * @throws OzoneClientException when uri is null or invalid volume name
+   */
+  protected String parseVolumeName(String uri) throws Exception{
+    URI ozoneURI = verifyURI(uri);
+    Path path = Paths.get(ozoneURI.getPath());
+    int pathNameCount = path.getNameCount();
+    if (pathNameCount != 1) {
+      String errorMessage;
+      if (pathNameCount < 1) {
+        errorMessage = "Volume name is required to perform volume " +
+            "operations like info, update, create and delete. ";
+      } else {
+        errorMessage = "Invalid volume name. Delimiters (/) not allowed in " +
+            "volume name";
+      }
+      throw new OzoneClientException(errorMessage);
+    }
+
+    return ozoneURI.getPath().replaceAll("^/+", "");
+  }
+
   public boolean isVerbose() {
     return parent.isVerbose();
   }

+ 1 - 11
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/DeleteVolumeHandler.java

@@ -18,9 +18,6 @@
 
 package org.apache.hadoop.ozone.web.ozShell.volume;
 
-import java.net.URI;
-
-import org.apache.hadoop.ozone.client.OzoneClientException;
 import org.apache.hadoop.ozone.web.ozShell.Handler;
 import org.apache.hadoop.ozone.web.ozShell.Shell;
 
@@ -43,14 +40,7 @@ public class DeleteVolumeHandler extends Handler {
   @Override
   public Void call() throws Exception {
 
-    URI ozoneURI = verifyURI(uri);
-    if (ozoneURI.getPath().isEmpty()) {
-      throw new OzoneClientException(
-          "Volume name is required to delete a volume");
-    }
-
-    // we need to skip the slash in the URI path
-    String volumeName = ozoneURI.getPath().substring(1);
+    String volumeName = parseVolumeName(uri);
 
     if (isVerbose()) {
       System.out.printf("Volume name : %s%n", volumeName);

+ 1 - 21
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/InfoVolumeHandler.java

@@ -18,11 +18,6 @@
 
 package org.apache.hadoop.ozone.web.ozShell.volume;
 
-import java.net.URI;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-
-import org.apache.hadoop.ozone.client.OzoneClientException;
 import org.apache.hadoop.ozone.client.OzoneClientUtils;
 import org.apache.hadoop.ozone.client.OzoneVolume;
 import org.apache.hadoop.ozone.web.ozShell.Handler;
@@ -48,22 +43,7 @@ public class InfoVolumeHandler extends Handler{
   @Override
   public Void call() throws Exception {
 
-    URI ozoneURI = verifyURI(uri);
-    Path path = Paths.get(ozoneURI.getPath());
-    int pathNameCount = path.getNameCount();
-    if (pathNameCount != 1) {
-      String errorMessage;
-      if (pathNameCount < 1) {
-        errorMessage = "Volume name is required to get info of a volume";
-      } else {
-        errorMessage = "Invalid volume name. Delimiters (/) not allowed in " +
-            "volume name";
-      }
-      throw new OzoneClientException(errorMessage);
-    }
-
-    // we need to skip the slash in the URI path
-    String volumeName = ozoneURI.getPath().substring(1);
+    String volumeName = parseVolumeName(uri);
 
     OzoneVolume vol = client.getObjectStore().getVolume(volumeName);
     System.out.printf("%s%n", JsonUtils.toJsonStringWithDefaultPrettyPrinter(

+ 1 - 11
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/volume/UpdateVolumeHandler.java

@@ -25,13 +25,10 @@ import picocli.CommandLine.Parameters;
 import org.apache.hadoop.hdds.client.OzoneQuota;
 import org.apache.hadoop.ozone.client.OzoneVolume;
 import org.apache.hadoop.ozone.client.OzoneClientUtils;
-import org.apache.hadoop.ozone.client.OzoneClientException;
 import org.apache.hadoop.ozone.web.ozShell.Handler;
 import org.apache.hadoop.ozone.web.ozShell.Shell;
 import org.apache.hadoop.ozone.web.utils.JsonUtils;
 
-import java.net.URI;
-
 /**
  * Executes update volume calls.
  */
@@ -57,14 +54,7 @@ public class UpdateVolumeHandler extends Handler {
   @Override
   public Void call() throws Exception {
 
-    URI ozoneURI = verifyURI(uri);
-    if (ozoneURI.getPath().isEmpty()) {
-      throw new OzoneClientException(
-          "Volume name is required to update a volume");
-    }
-
-    // we need to skip the slash in the URI path
-    String volumeName = ozoneURI.getPath().substring(1);
+    String volumeName = parseVolumeName(uri);
 
     OzoneVolume volume = client.getObjectStore().getVolume(volumeName);
     if (quota != null && !quota.isEmpty()) {