Skip to content

HDDS-11705. Snapshot operations on linked buckets should work on actual underlying bucket #7434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hemantk-12 merged 20 commits into from
Nov 16, 2024
Merged

HDDS-11705. Snapshot operations on linked buckets should work on actual underlying bucket #7434

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
62a2c1c
HDDS-11705. Snapshot diff fails on linked buckets
swamirishi Nov 14, 2024
2a3c55a
HDDS-11705. Snapshot operations should on linked buckets should work …
swamirishi Nov 15, 2024
77f69a2
HDDS-11705. Revert fixes made on snapshot flows
swamirishi Nov 15, 2024
df52e73
HDDS-11705. Address review comments
swamirishi Nov 15, 2024
3115158
HDDS-11705. Add test cases
swamirishi Nov 15, 2024
516f0a3
HDDS-11705. Address review comments
swamirishi Nov 15, 2024
7830c2b
HDDS-11705. Fix tests
swamirishi Nov 15, 2024
a286afc
HDDS-11705. Address review comments and move resolve check outside to…
swamirishi Nov 15, 2024
ce0fed0
HDDS-11705. Revert test case change
swamirishi Nov 15, 2024
50bc68b
HDDS-11705. Fix bats
swamirishi Nov 15, 2024
d638499
HDDS-11705. Fix bats
swamirishi Nov 15, 2024
82154a6
HDDS-11705. fix bats
swamirishi Nov 15, 2024
d0d5e91
HDDS-11705. Add linked bucket info in snapshot info
swamirishi Nov 15, 2024
029655d
HDDS-11705. Fix checkstyle
swamirishi Nov 15, 2024
e5353c7
HDDS-11705. Revert commented code
swamirishi Nov 15, 2024
0105fad
HDDS-11705. Revert renaming of WithoutLinkedBuckets tests
swamirishi Nov 15, 2024
798487e
HDDS-11705. Fix test case
swamirishi Nov 15, 2024
85daf58
HDDS-11705. Fix test case
swamirishi Nov 15, 2024
0bee4e3
HDDS-11705. Address review comments
swamirishi Nov 15, 2024
a861e5a
HDDS-11705. Fix build issue
swamirishi Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.UUID;

import com.google.common.cache.RemovalListener;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.hdds.StringUtils;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.server.ServerUtils;
Expand Down Expand Up @@ -656,7 +657,12 @@ private ReferenceCounted<OmSnapshot> getSnapshot(
// don't allow snapshot indicator without snapshot name
throw new OMException(INVALID_KEY_NAME);
}

// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(Pair.of(volumeName,
Comment thread
swamirishi marked this conversation as resolved.
Outdated
Comment thread
sumitagrawl marked this conversation as resolved.
Outdated
bucketName), false);
volumeName = resolvedBucket.realVolume();
bucketName = resolvedBucket.realBucket();
String snapshotTableKey = SnapshotInfo.getTableKey(volumeName,
bucketName, snapshotName);

Expand Down
30 changes: 21 additions & 9 deletions hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2968,6 +2968,11 @@ public ListKeysLightResult listKeysLight(String volumeName,
public SnapshotInfo getSnapshotInfo(String volumeName, String bucketName,
String snapshotName) throws IOException {
metrics.incNumSnapshotInfos();
// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
Comment thread
swamirishi marked this conversation as resolved.
Outdated
ResolvedBucket resolvedBucket = resolveBucketLink(Pair.of(volumeName, bucketName), false);
volumeName = resolvedBucket.realVolume();
Comment thread
swamirishi marked this conversation as resolved.
Outdated
bucketName = resolvedBucket.realBucket();
Map<String, String> auditMap = buildAuditMap(volumeName);
auditMap.put(OzoneConsts.BUCKET, bucketName);
try {
Expand All @@ -2994,6 +2999,11 @@ public ListSnapshotResponse listSnapshot(
String volumeName, String bucketName, String snapshotPrefix,
String prevSnapshot, int maxListResult) throws IOException {
metrics.incNumSnapshotLists();
// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
ResolvedBucket resolvedBucket = resolveBucketLink(Pair.of(volumeName, bucketName), false);
volumeName = resolvedBucket.realVolume();
bucketName = resolvedBucket.realBucket();
Map<String, String> auditMap = buildAuditMap(volumeName);
auditMap.put(OzoneConsts.BUCKET, bucketName);
try {
Expand Down Expand Up @@ -4898,10 +4908,11 @@ public SnapshotDiffResponse snapshotDiff(String volume,
boolean forceFullDiff,
boolean disableNativeDiff)
throws IOException {

if (isAclEnabled) {
omMetadataReader.checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, volume, bucket, null);
}
// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
ResolvedBucket resolvedBucket = resolveBucketLink(Pair.of(volume, bucket), false);
volume = resolvedBucket.realVolume();
bucket = resolvedBucket.realBucket();

return omSnapshotManager.getSnapshotDiffReport(volume, bucket, fromSnapshot, toSnapshot,
token, pageSize, forceFullDiff, disableNativeDiff);
Expand All @@ -4912,10 +4923,9 @@ public CancelSnapshotDiffResponse cancelSnapshotDiff(String volume,
String fromSnapshot,
String toSnapshot)
throws IOException {

if (isAclEnabled) {
omMetadataReader.checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.READ, volume, bucket, null);
}
ResolvedBucket resolvedBucket = this.resolveBucketLink(Pair.of(volume, bucket), false);
volume = resolvedBucket.realBucket();
bucket = resolvedBucket.realBucket();

return omSnapshotManager.cancelSnapshotDiff(volume, bucket, fromSnapshot, toSnapshot);
}
Expand All @@ -4925,7 +4935,9 @@ public List<SnapshotDiffJob> listSnapshotDiffJobs(String volume,
String jobStatus,
boolean listAll)
throws IOException {

ResolvedBucket resolvedBucket = this.resolveBucketLink(Pair.of(volume, bucket), false);
Comment thread
hemantk-12 marked this conversation as resolved.
volume = resolvedBucket.realBucket();
bucket = resolvedBucket.realBucket();
if (isAclEnabled) {
omMetadataReader.checkAcls(ResourceType.BUCKET, StoreType.OZONE, ACLType.LIST, volume, bucket, null);
}
Expand Down
14 changes: 11 additions & 3 deletions ...er/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@

package org.apache.hadoop.ozone.om.request.snapshot;

import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
import org.apache.hadoop.hdds.client.ReplicationConfig;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.utils.TransactionInfo;
import org.apache.hadoop.ozone.om.ResolvedBucket;
import org.apache.ratis.server.protocol.TermIndex;
import org.apache.hadoop.hdds.utils.db.RDBStore;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
Expand Down Expand Up @@ -75,8 +77,8 @@ public class OMSnapshotCreateRequest extends OMClientRequest {
LoggerFactory.getLogger(OMSnapshotCreateRequest.class);

private final String snapshotPath;
private final String volumeName;
private final String bucketName;
private String volumeName;
private String bucketName;
private final String snapshotName;
private final SnapshotInfo snapshotInfo;

Expand Down Expand Up @@ -106,7 +108,11 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
final OMRequest omRequest = super.preExecute(ozoneManager);
// Verify name
OmUtils.validateSnapshotName(snapshotName);

// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
ResolvedBucket bucket = ozoneManager.resolveBucketLink(Pair.of(volumeName, bucketName), this);
Comment thread
sumitagrawl marked this conversation as resolved.
this.volumeName = bucket.realVolume();
this.bucketName = bucket.realBucket();
UserGroupInformation ugi = createUGIForApi();
String bucketOwner = ozoneManager.getBucketOwner(volumeName, bucketName,
IAccessAuthorizer.ACLType.READ, OzoneObj.ResourceType.BUCKET);
Expand All @@ -120,6 +126,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
return omRequest.toBuilder().setCreateSnapshotRequest(
omRequest.getCreateSnapshotRequest().toBuilder()
.setSnapshotId(toProtobuf(UUID.randomUUID()))
.setVolumeName(volumeName)
.setBucketName(this.bucketName)
.setCreationTime(Time.now())
.build()).build();
}
Expand Down
7 changes: 7 additions & 0 deletions ...er/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package org.apache.hadoop.ozone.om.request.snapshot;

import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.ozone.om.ResolvedBucket;
import org.apache.ratis.server.protocol.TermIndex;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
Expand Down Expand Up @@ -82,6 +84,11 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {

String volumeName = deleteSnapshotRequest.getVolumeName();
String bucketName = deleteSnapshotRequest.getBucketName();
// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(Pair.of(volumeName, bucketName), this);
volumeName = resolvedBucket.realVolume();
bucketName = resolvedBucket.realBucket();

// Permission check
UserGroupInformation ugi = createUGIForApi();
Expand Down
8 changes: 8 additions & 0 deletions ...er/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotRenameRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@

import java.io.IOException;
import java.nio.file.InvalidPathException;

import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.audit.AuditLogger;
import org.apache.hadoop.ozone.audit.OMAction;
import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.ResolvedBucket;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.ozone.om.request.OMClientRequest;
Expand Down Expand Up @@ -75,6 +78,11 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {

String volumeName = renameSnapshotRequest.getVolumeName();
String bucketName = renameSnapshotRequest.getBucketName();
// Updating the volumeName & bucketName in case the bucket is a linked bucket. We need to do this before a
// permission check, since linked bucket permissions and source bucket permissions could be different.
ResolvedBucket resolvedBucket = ozoneManager.resolveBucketLink(Pair.of(volumeName, bucketName), this);
volumeName = resolvedBucket.realVolume();
bucketName = resolvedBucket.realBucket();

// Permission check
UserGroupInformation ugi = createUGIForApi();
Expand Down