Skip to content
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.

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

HDDS-1619. Support volume acl operations for OM HA. Contributed by… #1147

Merged
merged 16 commits into from
Aug 8, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@
import org.apache.hadoop.ozone.om.request.volume.OMVolumeDeleteRequest;
import org.apache.hadoop.ozone.om.request.volume.OMVolumeSetOwnerRequest;
import org.apache.hadoop.ozone.om.request.volume.OMVolumeSetQuotaRequest;
import org.apache.hadoop.ozone.om.request.volume.acl.OMVolumeAddAclRequest;
import org.apache.hadoop.ozone.om.request.volume.acl.OMVolumeRemoveAclRequest;
import org.apache.hadoop.ozone.om.request.volume.acl.OMVolumeSetAclRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
.OMRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneObj.ObjectType;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type;

Expand Down Expand Up @@ -117,12 +120,38 @@ public static OMClientRequest createClientRequest(OMRequest omRequest) {
return new S3MultipartUploadAbortRequest(omRequest);
case CompleteMultiPartUpload:
return new S3MultipartUploadCompleteRequest(omRequest);
case AddAcl:
case RemoveAcl:
case SetAcl:
return getOMAclRequest(omRequest);
default:
// TODO: will update once all request types are implemented.
return null;
}
}

private static OMClientRequest getOMAclRequest(OMRequest omRequest) {
Type cmdType = omRequest.getCmdType();
if (Type.AddAcl == cmdType) {
ObjectType type = omRequest.getAddAclRequest().getObj().getResType();
if (ObjectType.VOLUME == type) {
return new OMVolumeAddAclRequest(omRequest);
}
} else if (Type.RemoveAcl == cmdType) {
ObjectType type = omRequest.getAddAclRequest().getObj().getResType();
if (ObjectType.VOLUME == type) {
return new OMVolumeRemoveAclRequest(omRequest);
}
} else if (Type.SetAcl == cmdType) {
ObjectType type = omRequest.getAddAclRequest().getObj().getResType();
if (ObjectType.VOLUME == type) {
return new OMVolumeSetAclRequest(omRequest);
}
}
//TODO: handle bucket, key and prefix AddAcl
return null;
}

/**
* Convert exception result to {@link OzoneManagerProtocolProtos.Status}.
* @param exception
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package org.apache.hadoop.ozone.om.request.volume.acl;

import com.google.common.base.Optional;
import org.apache.hadoop.hdds.scm.storage.CheckedBiFunction;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OMMetrics;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
import org.apache.hadoop.ozone.om.request.OMClientRequest;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
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.apache.hadoop.utils.db.cache.CacheKey;
import org.apache.hadoop.utils.db.cache.CacheValue;

import java.io.IOException;
import java.util.List;

import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK;

/**
* Base class for OMVolumeAcl Request.
*/
public abstract class OMVolumeAclRequest extends OMClientRequest {

private CheckedBiFunction<List<OzoneAcl>, OmVolumeArgs, IOException>
omVolumeAclOp;

public OMVolumeAclRequest(OzoneManagerProtocolProtos.OMRequest omRequest,
CheckedBiFunction<List<OzoneAcl>, OmVolumeArgs, IOException> aclOp) {
super(omRequest);
omVolumeAclOp = aclOp;
}

@Override
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
long transactionLogIndex,
OzoneManagerDoubleBufferHelper ozoneManagerDoubleBufferHelper) {
// protobuf guarantees volume and acls are non-null.
String volume = getVolumeName();
List<OzoneAcl> ozoneAcls = getAcls();

OMMetrics omMetrics = ozoneManager.getMetrics();
omMetrics.incNumVolumeUpdates();
OmVolumeArgs omVolumeArgs = null;

OMResponse.Builder omResponse = onInit();
OMClientResponse omClientResponse = null;
IOException exception = null;

OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
boolean lockAcquired = false;
try {
// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE_ACL,
volume, null, null);
}
lockAcquired =
omMetadataManager.getLock().acquireLock(VOLUME_LOCK, volume);
String dbVolumeKey = omMetadataManager.getVolumeKey(volume);
omVolumeArgs = omMetadataManager.getVolumeTable().get(dbVolumeKey);
if (omVolumeArgs == null) {
throw new OMException(OMException.ResultCodes.VOLUME_NOT_FOUND);
}

// result is false upon add existing acl or remove non-existing acl
boolean result = true;
try {
omVolumeAclOp.apply(ozoneAcls, omVolumeArgs);
} catch (OMException ex) {
result = false;
}

if (result) {
// update cache.
omMetadataManager.getVolumeTable().addCacheEntry(
new CacheKey<>(dbVolumeKey),
new CacheValue<>(Optional.of(omVolumeArgs), transactionLogIndex));
}

omClientResponse = onSuccess(omResponse, omVolumeArgs, result);
} catch (IOException ex) {
exception = ex;
omMetrics.incNumVolumeUpdateFails();
omClientResponse = onFailure(omResponse, ex);
} finally {
if (omClientResponse != null) {
omClientResponse.setFlushFuture(
ozoneManagerDoubleBufferHelper.add(omClientResponse,
transactionLogIndex));
}
if (lockAcquired) {
omMetadataManager.getLock().releaseLock(VOLUME_LOCK, volume);
}
}

onComplete(exception);

return omClientResponse;
}

/**
* Get the Acls from the request.
* @return List of OzoneAcls, for add/remove it is a single element list
* for set it can be non-single element list.
*/
abstract List<OzoneAcl> getAcls();

/**
* Get the volume name from the request.
* @return volume name
* This is needed for case where volume does not exist and the omVolumeArgs is
* null.
*/
abstract String getVolumeName();

// TODO: Finer grain metrics can be moved to these callbacks. They can also
// be abstracted into separate interfaces in future.
/**
* Get the initial om response builder with lock.
* @return om response builder.
*/
abstract OMResponse.Builder onInit();

/**
* Get the om client response on success case with lock.
* @param omResponse
* @param omVolumeArgs
* @param result
* @return OMClientResponse
*/
abstract OMClientResponse onSuccess(
OMResponse.Builder omResponse, OmVolumeArgs omVolumeArgs, boolean result);

/**
* Get the om client response on failure case with lock.
* @param omResponse
* @param ex
* @return OMClientResponse
*/
abstract OMClientResponse onFailure(OMResponse.Builder omResponse,
IOException ex);

/**
* Completion hook for final processing before return without lock.
* Usually used for logging without lock.
* @param ex
*/
abstract void onComplete(IOException ex);
xiaoyuyao marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.ozone.om.request.volume.acl;

import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import org.apache.hadoop.hdds.scm.storage.CheckedBiFunction;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.volume.OMVolumeAclOpResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.List;

/**
* Handles volume add acl request.
*/
public class OMVolumeAddAclRequest extends OMVolumeAclRequest {
private static final Logger LOG =
LoggerFactory.getLogger(OMVolumeAddAclRequest.class);

private static CheckedBiFunction<List<OzoneAcl>,
OmVolumeArgs, IOException> volumeAddAclOp;

static {
volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
}

private List<OzoneAcl> ozoneAcls;
private String volumeName;

public OMVolumeAddAclRequest(OMRequest omRequest) {
super(omRequest, volumeAddAclOp);
OzoneManagerProtocolProtos.AddAclRequest addAclRequest =
getOmRequest().getAddAclRequest();
Preconditions.checkNotNull(addAclRequest);
ozoneAcls = Lists.newArrayList(
OzoneAcl.fromProtobuf(addAclRequest.getAcl()));
volumeName = addAclRequest.getObj().getPath().substring(1);
}

@Override
public List<OzoneAcl> getAcls() {
return ozoneAcls;
}

@Override
public String getVolumeName() {
return volumeName;
}

private OzoneAcl getAcl() {
return ozoneAcls.get(0);
}


@Override
OMResponse.Builder onInit() {
return OMResponse.newBuilder().setCmdType(
OzoneManagerProtocolProtos.Type.AddAcl)
.setStatus(OzoneManagerProtocolProtos.Status.OK).setSuccess(true);
}

@Override
OMClientResponse onSuccess(OMResponse.Builder omResponse,
OmVolumeArgs omVolumeArgs, boolean result){
omResponse.setAddAclResponse(OzoneManagerProtocolProtos.AddAclResponse
.newBuilder().setResponse(result).build());
return new OMVolumeAclOpResponse(omVolumeArgs, omResponse.build());
}

@Override
OMClientResponse onFailure(OMResponse.Builder omResponse,
IOException ex) {
return new OMVolumeAclOpResponse(null,
createErrorOMResponse(omResponse, ex));
}

@Override
void onComplete(IOException ex) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For onComplete we need to pass result also.
As in the case of already existing ACL, it is not a success.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is taken care in onSuccess.

Copy link
Contributor

@bharatviswa504 bharatviswa504 Aug 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh sorry for not being clear, in case of already existing ACL we have response set to false, and exception is null. But we still log as success.

Copy link
Contributor

@bharatviswa504 bharatviswa504 Aug 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, this is the reason we need to check response flag in OMVolumeAclResponse, as for false case, we don't need to do any operation in response addDBToBatch. Because even in that case our omResponse status is set to OK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should setSucess with operationResult flag, because onInit() sets it to true.

if (ex == null) {
LOG.debug("Add acl: {} to volume: {} success!",
getAcl(), getVolumeName());
} else {
LOG.error("Add acl {} to volume {} failed!",
getAcl(), getVolumeName(), ex);
}
}
}
xiaoyuyao marked this conversation as resolved.
Show resolved Hide resolved
Loading