[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-12-07 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1042388676


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -886,45 +1000,599 @@ public UpdateReservationHomeSubClusterResponse 
updateReservationHomeSubCluster(
 return UpdateReservationHomeSubClusterResponse.newInstance();
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Store NewMasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
   public RouterMasterKeyResponse storeNewMasterKey(RouterMasterKeyRequest 
request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+// Parse the delegationKey from the request and get the ZK storage path.
+DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+String nodeCreatePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+LOG.debug("Storing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+nodeCreatePath);
+
+// Write master key data to zk.
+try(ByteArrayOutputStream os = new ByteArrayOutputStream();

Review Comment:
   Space after try.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-12-06 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1041207295


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/TestSQLFederationStateStore.java:
##
@@ -592,4 +588,14 @@ public void testRemoveStoredToken() throws IOException, 
YarnException {
   public void testGetTokenByRouterStoreToken() throws IOException, 
YarnException {
 super.testGetTokenByRouterStoreToken();
   }
+
+  @Override
+  protected void checkRouterMasterKey(DelegationKey delegationKey,
+  RouterMasterKey routerMasterKey) throws YarnException, IOException {

Review Comment:
   Comment on why we do nothing



##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/TestSQLFederationStateStore.java:
##
@@ -18,21 +18,17 @@
 package org.apache.hadoop.yarn.server.federation.store.impl;
 
 import org.apache.commons.lang3.NotImplementedException;
+import org.apache.hadoop.security.token.delegation.DelegationKey;
 import org.apache.hadoop.test.LambdaTestUtils;
 import org.apache.hadoop.util.Time;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
 import org.apache.hadoop.yarn.api.records.ReservationId;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.exceptions.YarnException;
+import org.apache.hadoop.yarn.security.client.RMDelegationTokenIdentifier;
 import org.apache.hadoop.yarn.server.federation.store.FederationStateStore;
 import 
org.apache.hadoop.yarn.server.federation.store.metrics.FederationStateStoreClientMetrics;
-import org.apache.hadoop.yarn.server.federation.store.records.SubClusterId;
-import org.apache.hadoop.yarn.server.federation.store.records.SubClusterInfo;
-import 
org.apache.hadoop.yarn.server.federation.store.records.SubClusterRegisterRequest;
-import 
org.apache.hadoop.yarn.server.federation.store.records.ReservationHomeSubCluster;
-import 
org.apache.hadoop.yarn.server.federation.store.records.AddReservationHomeSubClusterRequest;
-import 
org.apache.hadoop.yarn.server.federation.store.records.UpdateReservationHomeSubClusterRequest;
-import 
org.apache.hadoop.yarn.server.federation.store.records.DeleteReservationHomeSubClusterRequest;
+import org.apache.hadoop.yarn.server.federation.store.records.*;

Review Comment:
   Avoid



##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/TestZookeeperFederationStateStore.java:
##
@@ -171,38 +203,117 @@ public void testMetricsInited() throws Exception {
 MetricsRecords.assertMetric(record, 
"UpdateReservationHomeSubClusterNumOps",  expectOps);
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test
   public void testStoreNewMasterKey() throws Exception {
 super.testStoreNewMasterKey();
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test

Review Comment:
   I think we can just inherit and no need to have them 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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-12-05 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1039698060


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/TestZookeeperFederationStateStore.java:
##
@@ -171,38 +203,293 @@ public void testMetricsInited() throws Exception {
 MetricsRecords.assertMetric(record, 
"UpdateReservationHomeSubClusterNumOps",  expectOps);
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test
   public void testStoreNewMasterKey() throws Exception {
-super.testStoreNewMasterKey();
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(1234, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);
+
+// Compare the data returned by the storeNewMasterKey
+// interface with the data queried by zk, and ensure that the data is 
consistent.
+RouterMasterKeyRequest routerMasterKeyRequest =
+RouterMasterKeyRequest.newInstance(paramRouterMasterKey);
+RouterMasterKeyResponse response = 
stateStore.storeNewMasterKey(routerMasterKeyRequest);
+assertNotNull(response);
+RouterMasterKey respRouterMasterKey = response.getRouterMasterKey();
+assertNotNull(respRouterMasterKey);
+
+// Get Data From zk.
+String nodeName = ROUTER_RM_DELEGATION_KEY_PREFIX + key.getKeyId();
+String nodePath = ZNODE_MASTER_KEY_PREFIX + nodeName;
+RouterMasterKey zkRouterMasterKey = getRouterMasterKeyFromZK(nodePath);
+
+assertNotNull(zkRouterMasterKey);
+assertEquals(paramRouterMasterKey, respRouterMasterKey);
+assertEquals(paramRouterMasterKey, zkRouterMasterKey);
+assertEquals(zkRouterMasterKey, respRouterMasterKey);
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test
   public void testGetMasterKeyByDelegationKey() throws YarnException, 
IOException {
-super.testGetMasterKeyByDelegationKey();
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(5678, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);
+
+// Call the getMasterKeyByDelegationKey interface of stateStore to get the 
MasterKey data.
+RouterMasterKeyRequest routerMasterKeyRequest =
+RouterMasterKeyRequest.newInstance(paramRouterMasterKey);
+RouterMasterKeyResponse response = 
stateStore.storeNewMasterKey(routerMasterKeyRequest);
+assertNotNull(response);
+
+// Get Data From zk.
+String nodeName = ROUTER_RM_DELEGATION_KEY_PREFIX + key.getKeyId();
+String nodePath = ZNODE_MASTER_KEY_PREFIX + nodeName;
+RouterMasterKey zkRouterMasterKey = getRouterMasterKeyFromZK(nodePath);
+
+// Call the getMasterKeyByDelegationKey interface to get the returned 
result.
+// The zk data should be consistent with the returned data.
+RouterMasterKeyResponse response1 =
+stateStore.getMasterKeyByDelegationKey(routerMasterKeyRequest);
+assertNotNull(response1);
+RouterMasterKey respRouterMasterKey = response1.getRouterMasterKey();
+assertEquals(paramRouterMasterKey, respRouterMasterKey);
+assertEquals(paramRouterMasterKey, zkRouterMasterKey);
+assertEquals(zkRouterMasterKey, respRouterMasterKey);
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test
   public void testRemoveStoredMasterKey() throws YarnException, IOException {
-super.testRemoveStoredMasterKey();
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(2345, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);
+
+// We need to ensure that the returned result is not empty.
+RouterMasterKeyRequest routerMasterKeyRequest =
+RouterMasterKeyRequest.newInstance(paramRouterMasterKey);
+RouterMasterKeyResponse response = 
stateStore.storeNewMasterKey(routerMasterKeyRequest);
+assertNotNull(response);
+
+// We will check if delegationToke

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-12-05 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1039696785


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/TestZookeeperFederationStateStore.java:
##
@@ -171,38 +203,293 @@ public void testMetricsInited() throws Exception {
 MetricsRecords.assertMetric(record, 
"UpdateReservationHomeSubClusterNumOps",  expectOps);
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test
   public void testStoreNewMasterKey() throws Exception {
-super.testStoreNewMasterKey();
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(1234, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);
+
+// Compare the data returned by the storeNewMasterKey
+// interface with the data queried by zk, and ensure that the data is 
consistent.
+RouterMasterKeyRequest routerMasterKeyRequest =
+RouterMasterKeyRequest.newInstance(paramRouterMasterKey);
+RouterMasterKeyResponse response = 
stateStore.storeNewMasterKey(routerMasterKeyRequest);
+assertNotNull(response);
+RouterMasterKey respRouterMasterKey = response.getRouterMasterKey();
+assertNotNull(respRouterMasterKey);
+
+// Get Data From zk.
+String nodeName = ROUTER_RM_DELEGATION_KEY_PREFIX + key.getKeyId();
+String nodePath = ZNODE_MASTER_KEY_PREFIX + nodeName;
+RouterMasterKey zkRouterMasterKey = getRouterMasterKeyFromZK(nodePath);
+
+assertNotNull(zkRouterMasterKey);
+assertEquals(paramRouterMasterKey, respRouterMasterKey);
+assertEquals(paramRouterMasterKey, zkRouterMasterKey);
+assertEquals(zkRouterMasterKey, respRouterMasterKey);
   }
 
-  @Test(expected = NotImplementedException.class)
+  @Test
   public void testGetMasterKeyByDelegationKey() throws YarnException, 
IOException {
-super.testGetMasterKeyByDelegationKey();
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(5678, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);

Review Comment:
   Can we use the super method and have a function to check these things we do 
in every method?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-11-30 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1036592621


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -886,45 +1000,608 @@ public UpdateReservationHomeSubClusterResponse 
updateReservationHomeSubCluster(
 return UpdateReservationHomeSubClusterResponse.newInstance();
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Store NewMasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
   public RouterMasterKeyResponse storeNewMasterKey(RouterMasterKeyRequest 
request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+// Parse the delegationKey from the request and get the ZK storage path.
+DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+String nodeCreatePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+LOG.debug("Storing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+nodeCreatePath);
+
+// Write master key data to zk.
+try(ByteArrayOutputStream os = new ByteArrayOutputStream();
+DataOutputStream fsOut = new DataOutputStream(os)) {
+  delegationKey.write(fsOut);
+  put(nodeCreatePath, os.toByteArray(), false);
+}
+
+// Get the stored masterKey from zk.
+RouterMasterKey masterKeyFromZK = getRouterMasterKeyFromZK(nodeCreatePath);
+long end = clock.getTime();
+opDurations.addStoreNewMasterKeyDuration(start, end);
+return RouterMasterKeyResponse.newInstance(masterKeyFromZK);
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Remove MasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
   public RouterMasterKeyResponse removeStoredMasterKey(RouterMasterKeyRequest 
request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+try {
+  // Parse the delegationKey from the request and get the ZK storage path.
+  RouterMasterKey masterKey = request.getRouterMasterKey();
+  DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+  String nodeRemovePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+  LOG.debug("Removing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+  nodeRemovePath);
+
+  // Check if the path exists, Throws an exception if the path does not 
exist.
+  if (!exists(nodeRemovePath)) {
+throw new YarnException("ZkNodePath = " + nodeRemovePath + " not 
exists!");
+  }
+
+  // try to remove masterKey.
+  zkManager.delete(nodeRemovePath);
+  long end = clock.getTime();
+  opDurations.removeStoredMasterKeyDuration(start, end);
+  return RouterMasterKeyResponse.newInstance(masterKey);
+} catch (Exception e) {
+  throw new YarnException(e);
+}
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Remove MasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
   public RouterMasterKeyResponse 
getMasterKeyByDelegationKey(RouterMasterKeyRequest request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+t

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-11-30 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1036514318


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/RouterStoreToken.java:
##
@@ -22,6 +22,7 @@
 import org.apache.hadoop.yarn.security.client.YARNDelegationTokenIdentifier;
 import org.apache.hadoop.yarn.util.Records;
 
+import java.io.DataInput;

Review Comment:
   Shouldn't that be covered in the impl and not in the super class?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-11-29 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1035037274


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/RouterStoreToken.java:
##
@@ -22,6 +22,7 @@
 import org.apache.hadoop.yarn.security.client.YARNDelegationTokenIdentifier;
 import org.apache.hadoop.yarn.util.Records;
 
+import java.io.DataInput;

Review Comment:
   Why do we need this? We didn't add anything using DataInput to this class.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-11-23 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1030718575


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -886,45 +975,554 @@ public UpdateReservationHomeSubClusterResponse 
updateReservationHomeSubCluster(
 return UpdateReservationHomeSubClusterResponse.newInstance();
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Store NewMasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
-  public RouterMasterKeyResponse storeNewMasterKey(RouterMasterKeyRequest 
request)
+  public synchronized RouterMasterKeyResponse 
storeNewMasterKey(RouterMasterKeyRequest request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+// Parse the delegationKey from the request and get the ZK storage path.
+DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+String nodeCreatePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+LOG.debug("Storing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+nodeCreatePath);
+
+// Write master key data to zk.
+try(ByteArrayOutputStream os = new ByteArrayOutputStream();
+DataOutputStream fsOut = new DataOutputStream(os)) {
+  delegationKey.write(fsOut);
+  put(nodeCreatePath, os.toByteArray(), false);
+}
+
+// Get the stored masterKey from zk.
+RouterMasterKey masterKeyFromZK = getRouterMasterKeyFromZK(nodeCreatePath);
+long end = clock.getTime();
+opDurations.addStoreNewMasterKeyDuration(start, end);
+return RouterMasterKeyResponse.newInstance(masterKeyFromZK);
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Remove MasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
-  public RouterMasterKeyResponse removeStoredMasterKey(RouterMasterKeyRequest 
request)
+  public synchronized RouterMasterKeyResponse 
removeStoredMasterKey(RouterMasterKeyRequest request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+try {
+  // Parse the delegationKey from the request and get the ZK storage path.
+  RouterMasterKey masterKey = request.getRouterMasterKey();
+  DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+  String nodeRemovePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+  LOG.debug("Removing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+  nodeRemovePath);
+
+  // Check if the path exists, Throws an exception if the path does not 
exist.
+  if (!exists(nodeRemovePath)) {
+throw new YarnException("ZkNodePath = " + nodeRemovePath + " not 
exists!");
+  }
+
+  // try to remove masterKey.
+  zkManager.delete(nodeRemovePath);
+  long end = clock.getTime();
+  opDurations.removeStoredMasterKeyDuration(start, end);
+  return RouterMasterKeyResponse.newInstance(masterKey);
+} catch (Exception e) {
+  throw new YarnException(e);
+}
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Remove MasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
   public RouterMasterKeyResponse 
getMasterKeyByDelegationKey(RouterMasterKeyRequest request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the re

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-11-22 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1029923406


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/FederationStateStoreBaseTest.java:
##
@@ -905,7 +905,7 @@ public void testGetMasterKeyByDelegationKey() throws 
YarnException, IOException
   }
 
   @Test
-  public void testRemoveStoredMasterKey() throws YarnException, IOException {
+  public void testRemoveStoredMasterKey() throws IOException, YarnException {

Review Comment:
   Avoid?



##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -886,45 +975,554 @@ public UpdateReservationHomeSubClusterResponse 
updateReservationHomeSubCluster(
 return UpdateReservationHomeSubClusterResponse.newInstance();
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Store NewMasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
-  public RouterMasterKeyResponse storeNewMasterKey(RouterMasterKeyRequest 
request)
+  public synchronized RouterMasterKeyResponse 
storeNewMasterKey(RouterMasterKeyRequest request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+// Parse the delegationKey from the request and get the ZK storage path.
+DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+String nodeCreatePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+LOG.debug("Storing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+nodeCreatePath);
+
+// Write master key data to zk.
+try(ByteArrayOutputStream os = new ByteArrayOutputStream();
+DataOutputStream fsOut = new DataOutputStream(os)) {
+  delegationKey.write(fsOut);
+  put(nodeCreatePath, os.toByteArray(), false);
+}
+
+// Get the stored masterKey from zk.
+RouterMasterKey masterKeyFromZK = getRouterMasterKeyFromZK(nodeCreatePath);
+long end = clock.getTime();
+opDurations.addStoreNewMasterKeyDuration(start, end);
+return RouterMasterKeyResponse.newInstance(masterKeyFromZK);
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Remove MasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+   * @return routerMasterKeyResponse, the response contains the 
RouterMasterKey.
+   * @throws YarnException if the call to the state store is unsuccessful.
+   * @throws IOException An IO Error occurred.
+   */
   @Override
-  public RouterMasterKeyResponse removeStoredMasterKey(RouterMasterKeyRequest 
request)
+  public synchronized RouterMasterKeyResponse 
removeStoredMasterKey(RouterMasterKeyRequest request)
   throws YarnException, IOException {
-throw new NotImplementedException("Code is not implemented");
+
+long start = clock.getTime();
+// For the verification of the request, after passing the verification,
+// the request and the internal objects will not be empty and can be used 
directly.
+FederationRouterRMTokenInputValidator.validate(request);
+
+try {
+  // Parse the delegationKey from the request and get the ZK storage path.
+  RouterMasterKey masterKey = request.getRouterMasterKey();
+  DelegationKey delegationKey = convertMasterKeyToDelegationKey(request);
+  String nodeRemovePath = 
getMasterKeyZNodePathByDelegationKey(delegationKey);
+  LOG.debug("Removing RMDelegationKey_{}, ZkNodePath = {}.", 
delegationKey.getKeyId(),
+  nodeRemovePath);
+
+  // Check if the path exists, Throws an exception if the path does not 
exist.
+  if (!exists(nodeRemovePath)) {
+throw new YarnException("ZkNodePath = " + nodeRemovePath + " not 
exists!");
+  }
+
+  // try to remove masterKey.
+  zkManager.delete(nodeRemovePath);
+  long end = clock.getTime();
+  opDurations.removeStoredMasterKeyDuration(start, end);
+  return RouterMasterKeyResponse.newInstance(masterKey);
+} catch (Exception e) {
+  throw new YarnException(e);
+}
   }
 
+  /**
+   * ZookeeperFederationStateStore Supports Remove MasterKey.
+   *
+   * @param request The request contains RouterMasterKey, which is an 
abstraction for DelegationKey
+

[GitHub] [hadoop] goiri commented on a diff in pull request #5131: YARN-11350. [Federation] Router Support DelegationToken With ZK.

2022-11-15 Thread GitBox 


goiri commented on code in PR #5131:
URL: https://github.com/apache/hadoop/pull/5131#discussion_r1023317011


##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/FederationStateStoreBaseTest.java:
##
@@ -879,7 +879,7 @@ public void testStoreNewMasterKey() throws Exception {
   }
 
   @Test
-  public void testGetMasterKeyByDelegationKey() throws YarnException, 
IOException {
+  public void testGetMasterKeyByDelegationKey() throws Exception {

Review Comment:
   This is correct but do we need to add all this churn?



##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##
@@ -121,6 +134,14 @@
  * |--- RESERVATION
  * | |- RESERVATION1
  * | |- RESERVATION2
+ * |--- ROUTER_RM_DT_SECRET_MANAGER_ROOT
+ * | |- ROUTER_RM_DELEGATION_TOKENS_ROOT
+ * | |   |- RM_DELEGATION_TOKEN_1
+ * | |   |- RM_DELEGATION_TOKEN_2
+ * | |   |- RM_DELEGATION_TOKEN_3
+ * | |- ROUTER_RM_DT_MASTER_KEYS_ROOT
+ * | |   |- DELEGATION_KEY_1
+ * |---  |- ROUTER_RM_DT_SEQUENTIAL_NUMBER

Review Comment:
   ```suggestion
* |  |- ROUTER_RM_DT_SEQUENTIAL_NUMBER
   ```



##
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/TestZookeeperFederationStateStore.java:
##
@@ -171,38 +202,270 @@ public void testMetricsInited() throws Exception {
 MetricsRecords.assertMetric(record, 
"UpdateReservationHomeSubClusterNumOps",  expectOps);
   }
 
-  @Test(expected = NotImplementedException.class)
   public void testStoreNewMasterKey() throws Exception {
-super.testStoreNewMasterKey();
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(1234, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);
+
+// Compare the data returned by the storeNewMasterKey
+// interface with the data queried by zk, and ensure that the data is 
consistent.
+RouterMasterKeyRequest routerMasterKeyRequest =
+RouterMasterKeyRequest.newInstance(paramRouterMasterKey);
+RouterMasterKeyResponse response = 
stateStore.storeNewMasterKey(routerMasterKeyRequest);
+assertNotNull(response);
+RouterMasterKey respRouterMasterKey = response.getRouterMasterKey();
+assertNotNull(respRouterMasterKey);
+
+// Get Data From zk.
+String nodeName = ROUTER_RM_DELEGATION_KEY_PREFIX + key.getKeyId();
+String nodePath = ZNODE_MASTER_KEY_PREFIX + nodeName;
+RouterMasterKey zkRouterMasterKey = getRouterMasterKeyFromZK(nodePath);
+
+assertNotNull(zkRouterMasterKey);
+assertEquals(paramRouterMasterKey, respRouterMasterKey);
+assertEquals(paramRouterMasterKey, zkRouterMasterKey);
+assertEquals(zkRouterMasterKey, respRouterMasterKey);
   }
 
-  @Test(expected = NotImplementedException.class)
-  public void testGetMasterKeyByDelegationKey() throws YarnException, 
IOException {
-super.testGetMasterKeyByDelegationKey();
+  public void testGetMasterKeyByDelegationKey() throws Exception {
+
+// Manually create a DelegationKey,
+// and call the interface storeNewMasterKey to write the data to zk.
+DelegationKey key = new DelegationKey(5678, Time.now() + 60 * 60, 
"keyBytes".getBytes());
+RouterMasterKey paramRouterMasterKey = 
RouterMasterKey.newInstance(key.getKeyId(),
+ByteBuffer.wrap(key.getEncodedKey()), key.getExpiryDate());
+FederationStateStore stateStore = this.getStateStore();
+
+assertTrue(stateStore instanceof ZookeeperFederationStateStore);
+
+// Call the getMasterKeyByDelegationKey interface of stateStore to get the 
MasterKey data.
+RouterMasterKeyRequest routerMasterKeyRequest =
+RouterMasterKeyRequest.newInstance(paramRouterMasterKey);
+RouterMasterKeyResponse response = 
stateStore.storeNewMasterKey(routerMasterKeyRequest);
+assertNotNull(response);
+
+// Get Data From zk.
+String nodeName = ROUTER_RM_DELEGATION_KEY_PREFIX + key.getKeyId();
+String nodePath = ZNODE_MASTER_KEY_PREFIX + nodeName;
+RouterMasterKey zkRouterMasterKey = getRouterMasterKeyFromZK(nodePath);
+
+// Call the getMasterKeyByDelegationKey interface to get the returned 
result.
+// The zk data should be consistent with the returned data.
+RouterMasterKeyResponse response1 =
+stateStore.getMasterKeyByDelegationKey(routerMasterKey