sarvekshayr commented on code in PR #11368:
URL: https://github.com/apache/ozone/pull/11368#discussion_r4193951801
##########
hadoop-ozone/dist/src/main/smoketest/basic/links.robot:
##########
@@ -94,6 +94,14 @@ Link to non-existent bucket
${result} = Execute And Ignore Error ozone sh key list
${target}/dangling-link
Should Contain ${result}
BUCKET_NOT_FOUND
+Link to non-existent source volume
+ ${missing} = Generate Random String 5 [NUMBERS]
+ ${missingVol} = Set Variable ${missing}-missing-source
+ Execute ozone sh bucket link
${missingVol}/any-bucket ${target}/dangling-missing-vol
Review Comment:
Lets simplify this -
```suggestion
Execute ozone sh bucket link
no-such-volume/no-such-bucket ${target}/dangling-missing-vol
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java:
##########
@@ -660,6 +662,60 @@ private static void denySourceRead(OmMetadataReader
metadataReader, String sourc
}).when(metadataReader).checkAcls(any(), any(), any(), any(), any(),
any());
}
+ @Test
+ void testResolveBucketLinkMissingSourceVolume() throws Exception {
+ String targetVolume = volumeName();
+ String missingSourceVolume = volumeName();
+ OmBucketInfo danglingLink = OmBucketInfo.newBuilder()
+ .setVolumeName(targetVolume)
+ .setBucketName("dangling-link")
+ .setSourceVolume(missingSourceVolume)
+ .setSourceBucket("any-bucket")
+ .build();
+ BucketManager bucketManager = mock(BucketManager.class);
+ when(bucketManager.getBucketInfo(targetVolume,
"dangling-link")).thenReturn(danglingLink);
+ when(bucketManager.getBucketInfo(missingSourceVolume, "any-bucket"))
+ .thenThrow(new OMException("Volume doesn't exist",
ResultCodes.VOLUME_NOT_FOUND));
+ OzoneManager omSpy = spy(omTestManagers.getOzoneManager());
+ HddsWhiteboxTestUtils.setInternalState(omSpy, "bucketManager",
bucketManager);
+ when(omSpy.getAclsEnabled()).thenReturn(false);
+
+ OMException omEx = assertThrows(OMException.class,
+ () -> omSpy.resolveBucketLink(Pair.of(targetVolume, "dangling-link")));
+ assertEquals(ResultCodes.BUCKET_NOT_FOUND, omEx.getResult());
+ assertTrue(omEx.getMessage().contains("Cannot follow bucket link"));
+ }
+
+ @Test
+ void testListKeysOnLinkWithMissingSourceVolume() throws Exception {
+ String targetVolume = volumeName();
+ String missingSourceVolume = volumeName();
+ OmBucketInfo danglingLink = OmBucketInfo.newBuilder()
+ .setVolumeName(targetVolume)
+ .setBucketName("dangling-link-list")
+ .setSourceVolume(missingSourceVolume)
+ .setSourceBucket("any-bucket")
+ .build();
+ BucketManager bucketManager = mock(BucketManager.class);
+ when(bucketManager.getBucketInfo(targetVolume,
"dangling-link-list")).thenReturn(danglingLink);
+ when(bucketManager.getBucketInfo(missingSourceVolume, "any-bucket"))
+ .thenThrow(new OMException("Volume doesn't exist",
ResultCodes.VOLUME_NOT_FOUND));
+ OzoneManager om = omTestManagers.getOzoneManager();
+ OzoneManager omSpy = spy(om);
+ HddsWhiteboxTestUtils.setInternalState(omSpy, "bucketManager",
bucketManager);
+ when(omSpy.getAclsEnabled()).thenReturn(false);
+ OmMetadataReader metadataReader = (OmMetadataReader)
HddsWhiteboxTestUtils.getInternalState(om,
+ "omMetadataReader");
+ HddsWhiteboxTestUtils.setInternalState(metadataReader, "ozoneManager",
omSpy);
+
+ OMException omEx = assertThrows(OMException.class,
+ () -> omSpy.listKeys(targetVolume, "dangling-link-list", null, null,
100));
+ assertEquals(ResultCodes.BUCKET_NOT_FOUND, omEx.getResult());
+ assertTrue(omEx.getMessage().contains("Cannot follow bucket link"));
Review Comment:
Use `assertThat` instead of `assertTrue` here as well.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -5416,6 +5418,13 @@ private OmBucketInfo resolveBucketLink(
if (allowDanglingBuckets) {
return null;
}
+
+ if (!visited.isEmpty()
+ && (e.getResult() == VOLUME_NOT_FOUND || e.getResult() ==
BUCKET_NOT_FOUND)) {
+ throw new OMException(
+ "Cannot follow bucket link: linked source bucket does not exist",
+ BUCKET_NOT_FOUND);
Review Comment:
Include source details to improve error message.
```suggestion
throw new OMException(
String.format("Cannot follow bucket link: linked source %s/%s
does not exist",
volumeName, bucketName),
e, BUCKET_NOT_FOUND);
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java:
##########
@@ -660,6 +662,60 @@ private static void denySourceRead(OmMetadataReader
metadataReader, String sourc
}).when(metadataReader).checkAcls(any(), any(), any(), any(), any(),
any());
}
+ @Test
+ void testResolveBucketLinkMissingSourceVolume() throws Exception {
+ String targetVolume = volumeName();
+ String missingSourceVolume = volumeName();
+ OmBucketInfo danglingLink = OmBucketInfo.newBuilder()
+ .setVolumeName(targetVolume)
+ .setBucketName("dangling-link")
+ .setSourceVolume(missingSourceVolume)
+ .setSourceBucket("any-bucket")
+ .build();
+ BucketManager bucketManager = mock(BucketManager.class);
+ when(bucketManager.getBucketInfo(targetVolume,
"dangling-link")).thenReturn(danglingLink);
+ when(bucketManager.getBucketInfo(missingSourceVolume, "any-bucket"))
+ .thenThrow(new OMException("Volume doesn't exist",
ResultCodes.VOLUME_NOT_FOUND));
+ OzoneManager omSpy = spy(omTestManagers.getOzoneManager());
+ HddsWhiteboxTestUtils.setInternalState(omSpy, "bucketManager",
bucketManager);
+ when(omSpy.getAclsEnabled()).thenReturn(false);
+
+ OMException omEx = assertThrows(OMException.class,
+ () -> omSpy.resolveBucketLink(Pair.of(targetVolume, "dangling-link")));
+ assertEquals(ResultCodes.BUCKET_NOT_FOUND, omEx.getResult());
+ assertTrue(omEx.getMessage().contains("Cannot follow bucket link"));
Review Comment:
Please use `assertThat` instead of `assertTrue`, see
[HDDS-9951](https://issues.apache.org/jira/browse/HDDS-9951).
```suggestion
assertThat(omEx.getMessage()).contains("Cannot follow bucket link");
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
