From 28ca5c9d1647837a1b2480d8935deffc6f68d807 Mon Sep 17 00:00:00 2001 From: Bharat Viswanadham Date: Fri, 12 Oct 2018 16:59:11 -0700 Subject: [PATCH] HDDS-613. Update HeadBucket, DeleteBucket to not to have volume in path. Contributed by Bharat Viswanadham. --- .../dist/src/main/smoketest/commonlib.robot | 7 ++++ .../dist/src/main/smoketest/s3/bucketv2.robot | 11 ++++++ .../dist/src/main/smoketest/s3/bucketv4.robot | 11 ++++++ .../src/main/smoketest/s3/commonawslib.robot | 5 +++ .../hadoop/ozone/s3/bucket/DeleteBucket.java | 7 ++-- .../hadoop/ozone/s3/bucket/HeadBucket.java | 9 ++--- .../hadoop/ozone/client/ObjectStoreStub.java | 36 +++++++++++++++++-- .../hadoop/ozone/client/OzoneVolumeStub.java | 12 +------ .../ozone/s3/bucket/TestDeleteBucket.java | 21 ++++------- .../ozone/s3/bucket/TestHeadBucket.java | 15 +++----- 10 files changed, 86 insertions(+), 48 deletions(-) diff --git a/hadoop-ozone/dist/src/main/smoketest/commonlib.robot b/hadoop-ozone/dist/src/main/smoketest/commonlib.robot index e2620fa434..eb3a8bb547 100644 --- a/hadoop-ozone/dist/src/main/smoketest/commonlib.robot +++ b/hadoop-ozone/dist/src/main/smoketest/commonlib.robot @@ -22,3 +22,10 @@ Execute Log ${output} Should Be Equal As Integers ${rc} 0 [return] ${output} + +Execute and checkrc + [arguments] ${command} ${expected_error_code} + ${rc} ${output} = Run And Return Rc And Output ${command} + Log ${output} + Should Be Equal As Integers ${rc} ${expected_error_code} + [return] ${output} \ No newline at end of file diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/bucketv2.robot b/hadoop-ozone/dist/src/main/smoketest/s3/bucketv2.robot index eb7fa452ec..f17189b41e 100644 --- a/hadoop-ozone/dist/src/main/smoketest/s3/bucketv2.robot +++ b/hadoop-ozone/dist/src/main/smoketest/s3/bucketv2.robot @@ -23,6 +23,7 @@ Resource commonawslib.robot ${ENDPOINT_URL} http://s3g:9878 ${OZONE_TEST} true ${BUCKET} generated +${NONEXIST-BUCKET} generated1 *** Keywords *** Install aws s3 cli @@ -53,3 +54,13 @@ Create Bucket Should contain ${result} Location Run Keyword if '${OZONE_TEST}' == 'true' Check Volume + +Head Bucket + ${result} = Execute AWSS3APICli head-bucket --bucket ${BUCKET} + ${result} = Execute AWSS3APICli and checkrc head-bucket --bucket ${NONEXIST-BUCKET} 255 + Should contain ${result} Not Found + Should contain ${result} 404 +Delete Bucket + ${result} = Execute AWSS3APICli head-bucket --bucket ${BUCKET} + ${result} = Execute AWSS3APICli and checkrc delete-bucket --bucket ${NONEXIST-BUCKET} 255 + Should contain ${result} NoSuchBucket \ No newline at end of file diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/bucketv4.robot b/hadoop-ozone/dist/src/main/smoketest/s3/bucketv4.robot index 5d306aad0a..1a93690da4 100644 --- a/hadoop-ozone/dist/src/main/smoketest/s3/bucketv4.robot +++ b/hadoop-ozone/dist/src/main/smoketest/s3/bucketv4.robot @@ -23,6 +23,7 @@ Resource commonawslib.robot ${ENDPOINT_URL} http://s3g:9878 ${OZONE_TEST} true ${BUCKET} generated +${NONEXIST-BUCKET} generated1 *** Keywords *** @@ -58,3 +59,13 @@ Create Bucket Should contain ${result} Location Run Keyword if '${OZONE_TEST}' == 'true' Check Volume + +Head Bucket + ${result} = Execute AWSS3APICli head-bucket --bucket ${BUCKET} + ${result} = Execute AWSS3APICli and checkrc head-bucket --bucket ${NONEXIST-BUCKET} 255 + Should contain ${result} Not Found + Should contain ${result} 404 +Delete Bucket + ${result} = Execute AWSS3APICli head-bucket --bucket ${BUCKET} + ${result} = Execute AWSS3APICli and checkrc delete-bucket --bucket ${NONEXIST-BUCKET} 255 + Should contain ${result} NoSuchBucket \ No newline at end of file diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/commonawslib.robot b/hadoop-ozone/dist/src/main/smoketest/s3/commonawslib.robot index be3a1b87f9..07fa66790e 100644 --- a/hadoop-ozone/dist/src/main/smoketest/s3/commonawslib.robot +++ b/hadoop-ozone/dist/src/main/smoketest/s3/commonawslib.robot @@ -20,4 +20,9 @@ Resource ../commonlib.robot Execute AWSS3APICli [Arguments] ${command} ${output} = Execute aws s3api --endpoint-url ${ENDPOINT_URL} ${command} + [return] ${output} + +Execute AWSS3APICli and checkrc + [Arguments] ${command} ${expected_error_code} + ${output} = Execute and checkrc aws s3api --endpoint-url ${ENDPOINT_URL} ${command} ${expected_error_code} [return] ${output} \ No newline at end of file diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/DeleteBucket.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/DeleteBucket.java index 3ccc4a2460..42885e21ac 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/DeleteBucket.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/DeleteBucket.java @@ -35,17 +35,16 @@ /** * Delete a bucket. */ -@Path("/{volume}/{bucket}") +@Path("/{bucket}") public class DeleteBucket extends EndpointBase { @DELETE @Produces(MediaType.APPLICATION_XML) - public Response delete(@PathParam("volume") String volumeName, - @PathParam("bucket") String bucketName) + public Response delete(@PathParam("bucket") String bucketName) throws IOException, OS3Exception { try { - getVolume(volumeName).deleteBucket(bucketName); + deleteS3Bucket(bucketName); } catch (IOException ex) { if (ex.getMessage().contains("BUCKET_NOT_EMPTY")) { OS3Exception os3Exception = S3ErrorTable.newError(S3ErrorTable diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/HeadBucket.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/HeadBucket.java index ba10e319e7..5ddc78cb25 100644 --- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/HeadBucket.java +++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/bucket/HeadBucket.java @@ -35,20 +35,17 @@ /** * Finds the bucket exists or not. */ -@Path("/{volume}/{bucket}") +@Path("/{bucket}") public class HeadBucket extends EndpointBase { private static final Logger LOG = LoggerFactory.getLogger(HeadBucket.class); @HEAD - public Response head(@PathParam("volume") String volumeName, - @PathParam("bucket") String bucketName) + public Response head(@PathParam("bucket") String bucketName) throws Exception { try { - getVolume(volumeName).getBucket(bucketName); - // Not sure what kind of error, we need to show for volume not exist - // to end user. As right now we throw run time exception. + getVolume(getOzoneVolumeName(bucketName)).getBucket(bucketName); } catch (IOException ex) { LOG.error("Exception occurred in headBucket", ex); if (ex.getMessage().contains("NOT_FOUND")) { diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ObjectStoreStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ObjectStoreStub.java index f0c52912e5..4984d58c32 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ObjectStoreStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/ObjectStoreStub.java @@ -37,6 +37,7 @@ public ObjectStoreStub() { private Map volumes = new HashMap<>(); private Map bucketVolumeMap = new HashMap<>(); + private Map bucketEmptyStatus = new HashMap<>(); @Override public void createVolume(String volumeName) throws IOException { @@ -112,25 +113,56 @@ public void deleteVolume(String volumeName) throws IOException { @Override public void createS3Bucket(String userName, String s3BucketName) throws IOException { - bucketVolumeMap.put(s3BucketName, "s3"+userName+"/"+s3BucketName); + if (bucketVolumeMap.get(s3BucketName) == null) { + String volumeName = "s3"+userName; + bucketVolumeMap.put(s3BucketName, volumeName + "/" + s3BucketName); + bucketEmptyStatus.put(s3BucketName, true); + createVolume(volumeName); + volumes.get(volumeName).createBucket(s3BucketName); + } else { + throw new IOException("BUCKET_ALREADY_EXISTS"); + } } @Override public void deleteS3Bucket(String s3BucketName) throws IOException { - bucketVolumeMap.remove(s3BucketName); + if (bucketVolumeMap.containsKey(s3BucketName)) { + if (bucketEmptyStatus.get(s3BucketName)) { + bucketVolumeMap.remove(s3BucketName); + } else { + throw new IOException("BUCKET_NOT_EMPTY"); + } + } else { + throw new IOException("BUCKET_NOT_FOUND"); + } } @Override public String getOzoneBucketMapping(String s3BucketName) throws IOException { + if (bucketVolumeMap.get(s3BucketName) == null) { + throw new IOException("S3_BUCKET_NOT_FOUND"); + } return bucketVolumeMap.get(s3BucketName); } + @Override public String getOzoneVolumeName(String s3BucketName) throws IOException { + if (bucketVolumeMap.get(s3BucketName) == null) { + throw new IOException("S3_BUCKET_NOT_FOUND"); + } return bucketVolumeMap.get(s3BucketName).split("/")[0]; } + @Override public String getOzoneBucketName(String s3BucketName) throws IOException { + if (bucketVolumeMap.get(s3BucketName) == null) { + throw new IOException("S3_BUCKET_NOT_FOUND"); + } return bucketVolumeMap.get(s3BucketName).split("/")[1]; } + + public void setBucketEmptyStatus(String bucketName, boolean status) { + bucketEmptyStatus.put(bucketName, status); + } } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java index c1104bf158..b1580e24da 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java @@ -35,7 +35,6 @@ public class OzoneVolumeStub extends OzoneVolume { private Map buckets = new HashMap<>(); - private Map bucketEmptyStatus = new HashMap<>(); public OzoneVolumeStub(String name, String admin, String owner, long quotaInBytes, @@ -61,7 +60,6 @@ public void createBucket(String bucketName, BucketArgs bucketArgs) bucketArgs.getStorageType(), bucketArgs.getVersioning(), System.currentTimeMillis())); - bucketEmptyStatus.put(bucketName, true); } @@ -98,17 +96,9 @@ public Iterator listBuckets(String bucketPrefix, @Override public void deleteBucket(String bucketName) throws IOException { if (buckets.containsKey(bucketName)) { - if (bucketEmptyStatus.get(bucketName)) { - buckets.remove(bucketName); - } else { - throw new IOException("BUCKET_NOT_EMPTY"); - } + buckets.remove(bucketName); } else { throw new IOException("BUCKET_NOT_FOUND"); } } - - public void setBucketEmptyStatus(String bucketName, boolean status) { - bucketEmptyStatus.put(bucketName, status); - } } diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestDeleteBucket.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestDeleteBucket.java index db9d68e934..513c33e7cc 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestDeleteBucket.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestDeleteBucket.java @@ -21,9 +21,8 @@ package org.apache.hadoop.ozone.s3.bucket; import org.apache.hadoop.ozone.client.ObjectStore; +import org.apache.hadoop.ozone.client.ObjectStoreStub; import org.apache.hadoop.ozone.client.OzoneClientStub; -import org.apache.hadoop.ozone.client.OzoneVolume; -import org.apache.hadoop.ozone.client.OzoneVolumeStub; import org.apache.hadoop.ozone.s3.exception.OS3Exception; import org.apache.hadoop.ozone.s3.exception.S3ErrorTable; import org.apache.http.HttpStatus; @@ -39,11 +38,9 @@ * This class tests delete bucket functionality. */ public class TestDeleteBucket { - private String volumeName = "myVolume"; private String bucketName = "myBucket"; private OzoneClientStub clientStub; private ObjectStore objectStoreStub; - private OzoneVolume volumeStub; private DeleteBucket deleteBucket; @Before @@ -53,11 +50,7 @@ public void setup() throws Exception { clientStub = new OzoneClientStub(); objectStoreStub = clientStub.getObjectStore(); - // Create volume and bucket - objectStoreStub.createVolume(volumeName); - - volumeStub = objectStoreStub.getVolume(volumeName); - volumeStub.createBucket(bucketName); + objectStoreStub.createS3Bucket("ozone", bucketName); // Create HeadBucket and setClient to OzoneClientStub deleteBucket = new DeleteBucket(); @@ -68,7 +61,7 @@ public void setup() throws Exception { @Test public void testDeleteBucket() throws Exception { - Response response = deleteBucket.delete(volumeName, bucketName); + Response response = deleteBucket.delete(bucketName); assertEquals(response.getStatus(), HttpStatus.SC_NO_CONTENT); } @@ -76,7 +69,7 @@ public void testDeleteBucket() throws Exception { @Test public void testDeleteWithNoSuchBucket() throws Exception { try { - deleteBucket.delete(volumeName, "unknownbucket"); + deleteBucket.delete("unknownbucket"); } catch (OS3Exception ex) { assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(), ex.getCode()); assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getErrorMessage(), @@ -91,10 +84,10 @@ public void testDeleteWithNoSuchBucket() throws Exception { public void testDeleteWithBucketNotEmpty() throws Exception { try { String bucket = "nonemptybucket"; - volumeStub.createBucket(bucket); - OzoneVolumeStub stub = (OzoneVolumeStub) volumeStub; + objectStoreStub.createS3Bucket("ozone1", bucket); + ObjectStoreStub stub = (ObjectStoreStub) objectStoreStub; stub.setBucketEmptyStatus(bucket, false); - deleteBucket.delete(volumeName, bucket); + deleteBucket.delete(bucket); } catch (OS3Exception ex) { assertEquals(S3ErrorTable.BUCKET_NOT_EMPTY.getCode(), ex.getCode()); assertEquals(S3ErrorTable.BUCKET_NOT_EMPTY.getErrorMessage(), diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestHeadBucket.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestHeadBucket.java index 8a79196d44..c392ac0e73 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestHeadBucket.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/bucket/TestHeadBucket.java @@ -22,7 +22,6 @@ import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneClientStub; -import org.apache.hadoop.ozone.client.OzoneVolume; import org.apache.hadoop.ozone.s3.exception.OS3Exception; import org.apache.hadoop.ozone.s3.exception.S3ErrorTable; import org.junit.Before; @@ -38,8 +37,8 @@ */ public class TestHeadBucket { - private String volumeName = "myVolume"; private String bucketName = "myBucket"; + private String userName = "ozone"; private OzoneClientStub clientStub; private ObjectStore objectStoreStub; private HeadBucket headBucket; @@ -51,23 +50,17 @@ public void setup() throws Exception { clientStub = new OzoneClientStub(); objectStoreStub = clientStub.getObjectStore(); - // Create volume and bucket - objectStoreStub.createVolume(volumeName); - - OzoneVolume volumeStub = objectStoreStub.getVolume(volumeName); - volumeStub.createBucket(bucketName); + objectStoreStub.createS3Bucket(userName, bucketName); // Create HeadBucket and setClient to OzoneClientStub headBucket = new HeadBucket(); headBucket.setClient(clientStub); - - } @Test public void testHeadBucket() throws Exception { - Response response = headBucket.head(volumeName, bucketName); + Response response = headBucket.head(bucketName); assertEquals(200, response.getStatus()); } @@ -75,7 +68,7 @@ public void testHeadBucket() throws Exception { @Test public void testHeadFail() { try { - headBucket.head(volumeName, "unknownbucket"); + headBucket.head("unknownbucket"); } catch (Exception ex) { if (ex instanceof OS3Exception) { assertEquals(S3ErrorTable.NO_SUCH_BUCKET.getCode(),