errose28 commented on a change in pull request #2857:
URL: https://github.com/apache/ozone/pull/2857#discussion_r759567327
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantDeleteRequest.java
##########
@@ -18,33 +18,256 @@
*/
package org.apache.hadoop.ozone.om.request.s3.tenant;
+import com.google.common.base.Optional;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMultiTenantManager;
import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmDBTenantInfo;
+import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.request.volume.OMVolumeRequest;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tenant.OMTenantDeleteResponse;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteTenantRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteVolumeRequest;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.ozone.storage.proto.OzoneManagerStorageProtos;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.METADATA_ERROR;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_EMPTY;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_FOUND;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_IS_REFERENCED;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_EMPTY;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.TENANT_LOCK;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK;
/**
* Handles OMTenantDelete request.
*/
public class OMTenantDeleteRequest extends OMVolumeRequest {
+ public static final Logger LOG =
+ LoggerFactory.getLogger(OMTenantDeleteRequest.class);
public OMTenantDeleteRequest(OMRequest omRequest) {
super(omRequest);
}
@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
- return getOmRequest();
+
+ // Check Ozone cluster admin privilege
+ OMTenantRequestHelper.checkAdmin(ozoneManager);
+
+ final OMMetadataManager metadataManager =
ozoneManager.getMetadataManager();
+ final DeleteTenantRequest request =
getOmRequest().getDeleteTenantRequest();
+ final String tenantId = request.getTenantId();
+
+ // Check tenant existence in tenantStateTable
+ if (!metadataManager.getTenantStateTable().isExist(tenantId)) {
+ LOG.debug("tenant: {} does not exist", tenantId);
+ throw new OMException("Tenant '" + tenantId + "' does not exist",
+ TENANT_NOT_FOUND);
+ }
+
+ final OMMultiTenantManager tenantManager =
+ ozoneManager.getMultiTenantManager();
+ // Check if there are any accessIds in the tenant
+ if (!OMTenantRequestHelper.isTenantEmpty(tenantManager, tenantId)) {
+ LOG.warn("tenant: '{}' is not empty. Unable to delete the tenant",
+ tenantId);
+ throw new OMException("Tenant '" + tenantId + "' is not empty. " +
+ "All accessIds associated to this tenant must be revoked before " +
+ "the tenant can be deleted.", TENANT_NOT_EMPTY);
+ }
+
+ // TODO: TBD: Call ozoneManager.getMultiTenantManager().deleteTenant() ?
+
+ // Regenerate request with the volumeName
+ final OmDBTenantInfo dbTenantInfo =
+ metadataManager.getTenantStateTable().get(tenantId);
+ final String volumeName = dbTenantInfo.getBucketNamespaceName();
+ final OMRequest.Builder omRequestBuilder = getOmRequest().toBuilder()
+ .setDeleteTenantRequest(DeleteTenantRequest.newBuilder()
+ .setTenantId(tenantId))
+ .setDeleteVolumeRequest(DeleteVolumeRequest.newBuilder()
+ .setVolumeName(volumeName))
+ // TODO: Can the three lines below be removed?
+ .setUserInfo(getUserInfo())
+ .setCmdType(getOmRequest().getCmdType())
+ .setClientId(getOmRequest().getClientId());
+
+ return omRequestBuilder.build();
}
@Override
+ @SuppressWarnings("methodlength")
public OMClientResponse validateAndUpdateCache(
OzoneManager ozoneManager, long transactionLogIndex,
OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
- return null;
+ OMClientResponse omClientResponse = null;
+ final OzoneManagerProtocolProtos.OMResponse.Builder omResponse =
+ OmResponseUtil.getOMResponseBuilder(getOmRequest());
+ boolean acquiredVolumeLock = false;
+ boolean acquiredUserLock = false;
+ boolean acquiredTenantLock = false;
+ final Map<String, String> auditMap = new HashMap<>();
+ OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+ final DeleteTenantRequest request =
getOmRequest().getDeleteTenantRequest();
+ final String tenantId = request.getTenantId();
+ // NOTE: volumeName might be a zero-length string
Review comment:
Is this still the case? I thought the client was explicitly setting the
volume name every time now.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantDeleteRequest.java
##########
@@ -18,33 +18,256 @@
*/
package org.apache.hadoop.ozone.om.request.s3.tenant;
+import com.google.common.base.Optional;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMultiTenantManager;
import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmDBTenantInfo;
+import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.request.volume.OMVolumeRequest;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tenant.OMTenantDeleteResponse;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteTenantRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteVolumeRequest;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.ozone.storage.proto.OzoneManagerStorageProtos;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.METADATA_ERROR;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_EMPTY;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_FOUND;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_IS_REFERENCED;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_EMPTY;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.TENANT_LOCK;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK;
/**
* Handles OMTenantDelete request.
*/
public class OMTenantDeleteRequest extends OMVolumeRequest {
+ public static final Logger LOG =
+ LoggerFactory.getLogger(OMTenantDeleteRequest.class);
public OMTenantDeleteRequest(OMRequest omRequest) {
super(omRequest);
}
@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
- return getOmRequest();
+
+ // Check Ozone cluster admin privilege
+ OMTenantRequestHelper.checkAdmin(ozoneManager);
+
+ final OMMetadataManager metadataManager =
ozoneManager.getMetadataManager();
+ final DeleteTenantRequest request =
getOmRequest().getDeleteTenantRequest();
+ final String tenantId = request.getTenantId();
+
+ // Check tenant existence in tenantStateTable
+ if (!metadataManager.getTenantStateTable().isExist(tenantId)) {
+ LOG.debug("tenant: {} does not exist", tenantId);
+ throw new OMException("Tenant '" + tenantId + "' does not exist",
+ TENANT_NOT_FOUND);
+ }
+
+ final OMMultiTenantManager tenantManager =
+ ozoneManager.getMultiTenantManager();
+ // Check if there are any accessIds in the tenant
+ if (!OMTenantRequestHelper.isTenantEmpty(tenantManager, tenantId)) {
Review comment:
This should be moved to validateAndUpdateCache as well, since this call
will need to check DB state.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantDeleteRequest.java
##########
@@ -18,33 +18,256 @@
*/
package org.apache.hadoop.ozone.om.request.s3.tenant;
+import com.google.common.base.Optional;
+import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
+import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.audit.OMAction;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OMMultiTenantManager;
import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.OmDBTenantInfo;
+import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
+import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.request.volume.OMVolumeRequest;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
+import org.apache.hadoop.ozone.om.response.s3.tenant.OMTenantDeleteResponse;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteTenantRequest;
+import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteVolumeRequest;
import
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+import org.apache.hadoop.ozone.storage.proto.OzoneManagerStorageProtos;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.METADATA_ERROR;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_EMPTY;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_FOUND;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_IS_REFERENCED;
+import static
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_EMPTY;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.TENANT_LOCK;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK;
+import static
org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK;
/**
* Handles OMTenantDelete request.
*/
public class OMTenantDeleteRequest extends OMVolumeRequest {
+ public static final Logger LOG =
+ LoggerFactory.getLogger(OMTenantDeleteRequest.class);
public OMTenantDeleteRequest(OMRequest omRequest) {
super(omRequest);
}
@Override
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
- return getOmRequest();
+
+ // Check Ozone cluster admin privilege
+ OMTenantRequestHelper.checkAdmin(ozoneManager);
+
+ final OMMetadataManager metadataManager =
ozoneManager.getMetadataManager();
+ final DeleteTenantRequest request =
getOmRequest().getDeleteTenantRequest();
+ final String tenantId = request.getTenantId();
+
+ // Check tenant existence in tenantStateTable
+ if (!metadataManager.getTenantStateTable().isExist(tenantId)) {
Review comment:
This is not safe. preExecute is not serialized, so just because the
tenant exists here, it may not exist when we do the get on line 103. We do need
to know the volume for this tenant, which requires reading the DB, so I think
the flow here should be refactored: Instead of appending a delete volume
request to the om request in preExecute, we can construct that same info in
validateAndUpdateCache and use that to delete the volume.
--
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]
