You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/03/16 13:50:43 UTC
[GitHub] [ozone] bharatviswa504 opened a new pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
bharatviswa504 opened a new pull request #2052:
URL: https://github.com/apache/ozone/pull/2052
## What changes were proposed in this pull request?
Use actual method parameter type when invoking a method, instead of deriving it from actual argument. (This will help when the actual arg param type is subclass, where as method param type is super class.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4985
## How was this patch tested?
Updated docker ozone-secure to run with ratis enabled.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#discussion_r617203023
##########
File path: hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh
##########
@@ -27,39 +27,42 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+for enable in true false; do
Review comment:
Thanks @elek It is added to test in a stand alone SCM cluster with ratis enable/disable.
As some of the integration issues were missed to identify in UTs. Later we have added a new acceptance test secure-ha, which tests end-to-end HA. I think now we can remove this if we think this is causing a longer CI duration. Thanks, @adoroszlai for opening a Jira to fix this.
(It would be good to have, but if we think it is expensive or any other way we can run this with ratis enable for single SCM cluster it would be good)
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#issuecomment-800899199
Thank You @bshashikant for the review.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#discussion_r617203023
##########
File path: hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh
##########
@@ -27,39 +27,42 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+for enable in true false; do
Review comment:
Thanks @elek It is added to test in a stand alone SCM cluster with ratis enable/disable.
As some of the integration issues were missed to identify in UTs. Later we have added a new acceptance test secure-ha, which tests end-to-end HA. (It would be good to have, but if we think it is expensive or any other way we can run this it would be good)
I think now we can remove this if we think this is causing a longer CI duration. Thanks, @adoroszlai for opening a Jira to fix this.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek commented on a change in pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#discussion_r617434787
##########
File path: hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh
##########
@@ -27,39 +27,42 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+for enable in true false; do
Review comment:
Thanks to explain it @bharatviswa504. I think it's too expensive to run ALL the tests in two modes. For example, we can assume that the linked and real buckets working at the same way as they already tested. We don't need to test all of them in both cases (IMHO).
I think it's fine to test different settings to make sure if it works. But we can adjust the required tests. For the main/default case we can execute all the tests and the other mode we can execute only a minimum set.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] GlenGeng commented on a change in pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on a change in pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#discussion_r595772986
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisRequest.java
##########
@@ -86,9 +94,14 @@ public Message encode() throws InvalidProtocolBufferException {
methodBuilder.setName(operation);
final List<MethodArgument> args = new ArrayList<>();
+
+ int paramCounter = 0;
for (Object argument : arguments) {
final MethodArgument.Builder argBuilder = MethodArgument.newBuilder();
- argBuilder.setType(argument.getClass().getName());
+ // Set actual method parameter type, not actual argument type.
+ // This is done to avoid MethodNotFoundException in case if argument is
Review comment:
Nice change!
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 edited a comment on pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#issuecomment-800899199
Thank You @bshashikant and @GlenGeng for the review.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek commented on a change in pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2052:
URL: https://github.com/apache/ozone/pull/2052#discussion_r616570112
##########
File path: hadoop-ozone/dist/src/main/compose/ozonesecure/test.sh
##########
@@ -27,39 +27,42 @@ export SECURITY_ENABLED=true
: ${OZONE_BUCKET_KEY_NAME:=key1}
-start_docker_env
+for enable in true false; do
Review comment:
This trick doubled the execution time of the secure acceptance test which was already 50 min. It increased the full build time with ~50 mins and (because the parallel execution assumed all the tests finish in 1 hour) the full build is also slower with 30-40 minutes.
Do we really need this trick?
(By the way the `enable` variable name is not very expressive, and it should be added to the `-N` because different tests will overwrite the results of each other)
cc @adoroszlai @bharatviswa504
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bharatviswa504 merged pull request #2052: HDDS-4985. [SCM HA Security] When Ratis enable, SCM secure cluster is not working.
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #2052:
URL: https://github.com/apache/ozone/pull/2052
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org