smengcl commented on a change in pull request #2857:
URL: https://github.com/apache/ozone/pull/2857#discussion_r765250238



##########
File path: 
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/tenant/OMTenantDeleteRequest.java
##########
@@ -18,33 +18,206 @@
  */
 package org.apache.hadoop.ozone.om.request.s3.tenant;
 
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+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.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.DeleteTenantResponse;
 import 
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
+import 
org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.hadoop.ozone.security.acl.OzoneObj;
+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.TENANT_NOT_EMPTY;
+import static 
org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TENANT_NOT_FOUND;
+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);
+
+    // TODO: TBD: Call ozoneManager.getMultiTenantManager().deleteTenant() ?
+
+    return getOmRequest().toBuilder().setUserInfo(getUserInfo()).build();
   }
 
   @Override
+  @SuppressWarnings("methodlength")
   public OMClientResponse validateAndUpdateCache(
       OzoneManager ozoneManager, long transactionLogIndex,
       OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
 
-    return null;
+    OMClientResponse omClientResponse = null;
+    final OMResponse.Builder omResponse =
+        OmResponseUtil.getOMResponseBuilder(getOmRequest());
+    boolean acquiredVolumeLock = false;
+    final Map<String, String> auditMap = new HashMap<>();
+    OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
+    final DeleteTenantRequest request = 
getOmRequest().getDeleteTenantRequest();
+    final String tenantId = request.getTenantId();
+    String volumeName = null;
+    boolean decVolumeRefCount = true;
+
+    IOException exception = null;
+    OmVolumeArgs omVolumeArgs = null;
+
+    try {
+      // Check tenant existence in tenantStateTable
+      if (!omMetadataManager.getTenantStateTable().isExist(tenantId)) {
+        LOG.debug("tenant: {} does not exist", tenantId);
+        throw new OMException("Tenant '" + tenantId + "' does not exist",
+            TENANT_NOT_FOUND);
+      }
+
+      // Reading the TenantStateTable without lock as we don't have or need
+      // a TENANT_LOCK. The assumption is that OmDBTenantInfo is read-only
+      // once it is set during tenant creation.
+      final OmDBTenantInfo dbTenantInfo =
+          omMetadataManager.getTenantStateTable().get(tenantId);
+      volumeName = dbTenantInfo.getBucketNamespaceName();
+      assert(volumeName != null);
+
+      LOG.debug("Tenant '{}' has volume '{}'", tenantId, volumeName);
+      // decVolumeRefCount is true if volumeName is not empty string
+      decVolumeRefCount = volumeName.length() > 0;
+
+      // Acquire the volume lock
+      Preconditions.checkNotNull(volumeName,

Review comment:
       This really is an assertion. So that in some other potential buggy write 
path we could catch it asap (instead of propagating it further). I guess now 
that we won't have UpdateTenant, CreateTenant is indeed the only write path, 
for now. Unless the DB is tampered with, we shouldn't encounter `volumeName == 
null` at all.
   
   Also I just realized I already had a `assert(volumeName != null)` earlier. 
So I'm just going to remove this line. (though assert keyword doesn't trigger 
in prod)




-- 
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]

Reply via email to