[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r309526798
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -0,0 +1,103 @@
+/**
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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){
+LOG.debug("Add acl: {} to volume: {} success!",
+getAcl(), getVolumeName());
+omResponse.setAddAclResponse(OzoneManagerProtocolProtos.AddAclResponse
+.newBuilder().setResponse(true).build());
+return new OMVolumeAclOpResponse(omVolumeArgs, omResponse.build());
+ }
+
+ @Override
+ OMClientResponse onFailure(OMResponse.Builder omResponse,
+ IOException ex) {
+LOG.error("Add acl {} to volume {} failed!",
Review comment:
I liked this approach, code looks cleaner.
Can we move logging outside of these onSuccess and OnFailure. As these are
called with lock held.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r309527672
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerHARequestHandlerImpl.java
##
@@ -70,6 +70,7 @@ public OMResponse handleApplyTransaction(OMRequest omRequest,
case CreateS3Bucket:
case DeleteS3Bucket:
case InitiateMultiPartUpload:
+case AddAcl:
Review comment:
Yes, But when addAcl/removeAcl/setAcl comes for bucket which is not handled,
so for them we should call normal handler.handle. So, here we need to have
below code
`
if (omClientRequest != null) { OMClientResponse omClientResponse =
omClientRequest.validateAndUpdateCache(getOzoneManager(),
transactionLogIndex, ozoneManagerDoubleBuffer::add);
} else {
return handler.handle
}`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r309527672
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerHARequestHandlerImpl.java
##
@@ -70,6 +70,7 @@ public OMResponse handleApplyTransaction(OMRequest omRequest,
case CreateS3Bucket:
case DeleteS3Bucket:
case InitiateMultiPartUpload:
+case AddAcl:
Review comment:
Yes, But when addAcl/removeAcl/setAcl comes for bucket which is not handled,
so for them we should call normal handler.handle. So, here we need to have
below code
`
if (omClientRequest != null) { OMClientResponse omClientResponse =
omClientRequest.validateAndUpdateCache(getOzoneManager(),
transactionLogIndex, ozoneManagerDoubleBuffer::add);
} else {
return handle(omClientRequest);
}`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by… URL: https://github.com/apache/hadoop/pull/1147#discussion_r309528209 ## File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/TestOMVolumeAddAclRequest.java ## @@ -0,0 +1,123 @@ +/** + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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; + +import org.apache.hadoop.ozone.OzoneAcl; +import org.apache.hadoop.ozone.om.helpers.OmOzoneAclMap; +import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; +import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; +import org.apache.hadoop.ozone.om.request.volume.acl.OMVolumeAddAclRequest; +import org.apache.hadoop.ozone.om.response.OMClientResponse; +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.junit.Assert; +import org.junit.Test; + +import java.util.UUID; + Review comment: all acl tests should be in package volume/acl like same as in source. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r309979373
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
For onComplete we need to pass result also.
As in the case of already existing ACL, it is not a success.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r309979423
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAclRequest.java
##
@@ -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, OmVolumeArgs, IOException>
+ omVolumeAclOp;
+
+ public OMVolumeAclRequest(OzoneManagerProtocolProtos.OMRequest omRequest,
+ CheckedBiFunction, 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 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 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
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r309980204
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/volume/OMVolumeAclOpResponse.java
##
@@ -0,0 +1,63 @@
+/**
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.response.volume;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
+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.utils.db.BatchOperation;
+
+import java.io.IOException;
+
+/**
+ * Response for om volume acl operation request.
+ */
+public class OMVolumeAclOpResponse extends OMClientResponse {
+
+ private OmVolumeArgs omVolumeArgs;
+
+ public OMVolumeAclOpResponse(OmVolumeArgs omVolumeArgs,
+ OMResponse omResponse) {
+super(omResponse);
+this.omVolumeArgs = omVolumeArgs;
+ }
+
+ @Override
+ public void addToDBBatch(OMMetadataManager omMetadataManager,
+ BatchOperation batchOperation) throws IOException {
+
+// For OmResponse with failure, this should do nothing. This method is
+// not called in failure scenario in OM code.
+if (getOMResponse().getStatus() == OzoneManagerProtocolProtos.Status.OK) {
+ omMetadataManager.getVolumeTable().putWithBatch(batchOperation,
Review comment:
Here we need to check the OMResponse flag also. As for existing acl, we
should set OMResponse response to false. And in that case, we don't need
anything to be added to DB.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r310339907
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
Ohh sorry for not being clear, in case of already existing ACL we have
response set to false, and exception is null. BBut we still log as success.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r310339929
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
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.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r310339929
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
so, this is the reason we need to check response flag in
OMVolumeAclResponse, as for false case, we don't need to do any op in response
addDBToBatch.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r310339929
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
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.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r310339987
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
https://github.com/apache/hadoop/pull/1202/files#diff-434a93a70678f5ddbff01e2e7445ac38R47
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r310339907
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
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.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[GitHub] [hadoop] bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support volume acl operations for OM HA. Contributed by…
bharatviswa504 commented on a change in pull request #1147: HDDS-1619. Support
volume acl operations for OM HA. Contributed by…
URL: https://github.com/apache/hadoop/pull/1147#discussion_r311318331
##
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/acl/OMVolumeAddAclRequest.java
##
@@ -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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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,
+ OmVolumeArgs, IOException> volumeAddAclOp;
+
+ static {
+volumeAddAclOp = (acls, volArgs) -> volArgs.addAcl(acls.get(0));
+ }
+
+ private List 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 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) {
Review comment:
We should setSucess with operationResult flag, because onInit() sets it to
true.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
