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 2020/04/02 01:24:48 UTC

[GitHub] [hadoop-ozone] xiaoyuyao opened a new pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

xiaoyuyao opened a new pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752
 
 
   ## What changes were proposed in this pull request?
   Fix the token renew and cancel logic in the Renewer class. By adding omserviceId into OzoneTokenIdentifier. This way, the renewer can renew and cancel the token without referring any additional ozone uri.
    
   Fix the ozone token tool to support get/renew/cancel token in secure om ha setup.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-3319
   
   ## How was this patch tested?
   Test with the private build in real cluster using ozone token tool and the distcp. 
   Before the fix, the YARN RM will stuck due to the omservice id is not being used to connect to the om proxy up job submit to renew the ozone token. After the fix, the distcp will be able to finish successfully. 
   
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752
 
 
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402056499
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/token/GetTokenHandler.java
 ##########
 @@ -39,7 +40,9 @@
     description = "get a delegation token.")
 public class GetTokenHandler extends Handler {
 
-
+  @CommandLine.Parameters(arity = "1..1",
+      description = Shell.OZONE_BUCKET_URI_DESCRIPTION)
 
 Review comment:
   OZONE_BUCKET_URI_DESCRIPTION -> need to be updated as o3://om or o3://serviceid

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402057401
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
 
 Review comment:
   Do we need to update it to something like this  if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT) && !OmUtils.isOmHAServiceId(conf, omServiceId))

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402088165
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
+        // Non-HA
+        return OzoneClientFactory.getRpcClient(conf);
+      } else if (OmUtils.isOmHAServiceId(conf, omServiceId)) {
+        // HA with matching service id
+        return OzoneClientFactory.getRpcClient(omServiceId, conf);
+      } else {
+        // HA with mismatched service id
+        throw new IOException("Service ID specified does not match" +
 
 Review comment:
   good point, will fix in the next commit.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402056499
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/token/GetTokenHandler.java
 ##########
 @@ -39,7 +40,9 @@
     description = "get a delegation token.")
 public class GetTokenHandler extends Handler {
 
-
+  @CommandLine.Parameters(arity = "1..1",
+      description = Shell.OZONE_BUCKET_URI_DESCRIPTION)
 
 Review comment:
   OZONE_BUCKET_URI_DESCRIPTION -> need to be updated as o3://om or o3://<<serviceid>>

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402050759
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
+        // Non-HA
+        return OzoneClientFactory.getRpcClient(conf);
+      } else if (OmUtils.isOmHAServiceId(conf, omServiceId)) {
+        // HA with matching service id
+        return OzoneClientFactory.getRpcClient(omServiceId, conf);
+      } else {
+        // HA with mismatched service id
+        throw new IOException("Service ID specified does not match" +
+            " with " + OZONE_OM_SERVICE_IDS_KEY + " defined in the " +
+            "configuration. Configured " + OZONE_OM_SERVICE_IDS_KEY +
+            " are" + conf.getTrimmedStringCollection(
+            OZONE_OM_SERVICE_IDS_KEY));
+      }
+    } else {
+      // Old OM may issue token without omServiceId that should work
+      // with non-HA case
+      if (!OmUtils.isServiceIdsDefined(conf)) {
+        return OzoneClientFactory.getRpcClient(conf);
+      } else {
 
 Review comment:
   This else, says when new client talking to old OM with HA, we are failing is my understanding correct here?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402058366
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
+        // Non-HA
+        return OzoneClientFactory.getRpcClient(conf);
+      } else if (OmUtils.isOmHAServiceId(conf, omServiceId)) {
+        // HA with matching service id
+        return OzoneClientFactory.getRpcClient(omServiceId, conf);
+      } else {
+        // HA with mismatched service id
+        throw new IOException("Service ID specified does not match" +
 
 Review comment:
   Minor: Can we also found service ID which helps during debug

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402049821
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
 
 Review comment:
   Now with this change, we have an assumption that client can never use OM_SERVICE_ID_DEFAULT value when configuring HA. As previously in single node OM with ratis enabled, we need raftGroupID to get that we have used this default serviceID.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402095314
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/token/GetTokenHandler.java
 ##########
 @@ -39,7 +40,9 @@
     description = "get a delegation token.")
 public class GetTokenHandler extends Handler {
 
-
+  @CommandLine.Parameters(arity = "1..1",
+      description = Shell.OZONE_BUCKET_URI_DESCRIPTION)
 
 Review comment:
   Fixed.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402087161
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
+        // Non-HA
+        return OzoneClientFactory.getRpcClient(conf);
+      } else if (OmUtils.isOmHAServiceId(conf, omServiceId)) {
+        // HA with matching service id
+        return OzoneClientFactory.getRpcClient(omServiceId, conf);
+      } else {
+        // HA with mismatched service id
+        throw new IOException("Service ID specified does not match" +
+            " with " + OZONE_OM_SERVICE_IDS_KEY + " defined in the " +
+            "configuration. Configured " + OZONE_OM_SERVICE_IDS_KEY +
+            " are" + conf.getTrimmedStringCollection(
+            OZONE_OM_SERVICE_IDS_KEY));
+      }
+    } else {
+      // Old OM may issue token without omServiceId that should work
+      // with non-HA case
+      if (!OmUtils.isServiceIdsDefined(conf)) {
+        return OzoneClientFactory.getRpcClient(conf);
+      } else {
 
 Review comment:
   The serviceID must be unique to support distcp cross clusters. In the case when token does not provide omServiceId or only the default serviceID is provided. Token renew/cancel may not be able to figure out the right OM clusters to talk to. That's the main reason that we fail here. 

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #752: HDDS-3319. Handle HA for BasicOzoneClientAdapterImpl#renew/cancel().
URL: https://github.com/apache/hadoop-ozone/pull/752#discussion_r402090427
 
 

 ##########
 File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientFactory.java
 ##########
 @@ -159,6 +163,47 @@ private static OzoneClient getRpcClient(ClientProtocol clientProtocol,
     return new OzoneClient(config, proxy);
   }
 
+  /**
+   * Create OzoneClient for token renew/cancel operations.
+   * @param conf Configuration to be used for OzoneCient creation
+   * @param token ozone token is involved
+   * @return
+   * @throws IOException
+   */
+  public static OzoneClient getOzoneClient(Configuration conf,
+      Token<OzoneTokenIdentifier> token) throws IOException {
+    Preconditions.checkNotNull(token, "Null token is not allowed");
+    String omServiceId = token.decodeIdentifier().getOmServiceId();
+    if (StringUtils.isNotEmpty(omServiceId)) {
+      // new OM should always issue token with omServiceId
+      if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
 
 Review comment:
   Do we use the OM_SERVICE_ID_DEFAULT for non-HA case? My first version does not have the following logic and I thought this is purely for non-HA where we don't need this serviceID to create RPC client. 
   
   if (omServiceId.equals(OzoneConsts.OM_SERVICE_ID_DEFAULT)) {
           // Non-HA
           return OzoneClientFactory.getRpcClient(conf);
         }
   
   If all the HA and HA are unified, we can skip those lines and only use the second if when the omServiceId match with the local configure. Still, the default om service id is risky in cross cluster operations like distcp as I mentioned below. 
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org