Skip to content

Commit 5141147

Browse files
slfan1989slfan1989
committed
YARN-11267. Improve Some Code.
1 parent 7a8b819 commit 5141147

File tree

4 files changed

+71
-25
lines changed

4 files changed

+71
-25
lines changed

‎hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/main/java/org/apache/hadoop/yarn/server/router/webapp/FederationInterceptorREST.java‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,8 @@
2222
import java.lang.reflect.Method;
2323
import java.security.Principal;
2424
import java.security.PrivilegedExceptionAction;
25-
import java.util.ArrayList;
26-
import java.util.Collection;
27-
import java.util.HashMap;
28-
import java.util.List;
29-
import java.util.Map;
25+
import java.util.*;
3026
import java.util.Map.Entry;
31-
import java.util.Set;
3227
import java.util.concurrent.CompletionService;
3328
import java.util.concurrent.ExecutorCompletionService;
3429
import java.util.concurrent.ExecutorService;
@@ -140,6 +135,7 @@
140135
import org.apache.hadoop.yarn.server.webapp.dao.ContainerInfo;
141136
import org.apache.hadoop.yarn.server.webapp.dao.ContainersInfo;
142137
import org.apache.hadoop.yarn.util.LRUCacheHashMap;
138+
import org.apache.hadoop.yarn.webapp.ForbiddenException;
143139
import org.apache.hadoop.yarn.webapp.dao.ConfInfo;
144140
import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
145141
import org.apache.hadoop.yarn.util.Clock;
@@ -1001,8 +997,8 @@ private SubClusterInfo getNodeSubcluster(String nodeId) throws NotFoundException
1001997
NodeInfo nodeInfo = null;
1002998
for (Entry<SubClusterInfo, NodeInfo> entry : results.entrySet()) {
1003999
NodeInfo nodeResponse = entry.getValue();
1004-
if (nodeInfo == null || nodeInfo.getLastHealthUpdate() <
1005-
nodeResponse.getLastHealthUpdate()) {
1000+
if (nodeInfo == null ||(nodeInfo != null && nodeResponse != null && nodeInfo.getLastHealthUpdate() <
1001+
nodeResponse.getLastHealthUpdate())) {
10061002
subcluster = entry.getKey();
10071003
nodeInfo = nodeResponse;
10081004
}
@@ -1137,6 +1133,7 @@ public AppState getAppState(HttpServletRequest hsr, String appId)
11371133
}
11381134
} catch (YarnException | IllegalArgumentException e) {
11391135
LOG.error("getHomeSubClusterInfoByAppId error, applicationId = {}.", appId, e);
1136+
return null;
11401137
}
11411138
return new AppState();
11421139
}
@@ -1385,8 +1382,8 @@ public ActivitiesInfo getActivities(HttpServletRequest hsr, String nodeId,
13851382
String groupBy) {
13861383
try {
13871384
// Check the parameters to ensure that the parameters are not empty
1388-
// Validate.checkNotNullAndNotEmpty(nodeId, "nodeId");
1389-
// Validate.checkNotNullAndNotEmpty(groupBy, "groupBy");
1385+
Validate.checkNotNullAndNotEmpty(nodeId, "nodeId");
1386+
Validate.checkNotNullAndNotEmpty(groupBy, "groupBy");
13901387

13911388
// Query SubClusterInfo according to id,
13921389
// if the nodeId cannot get SubClusterInfo, an exception will be thrown directly.
@@ -3356,11 +3353,22 @@ private <R> Map<SubClusterInfo, R> invokeConcurrent(Collection<SubClusterInfo> c
33563353
} catch (Exception e) {
33573354
LOG.error("SubCluster {} failed to call {} method.",
33583355
info.getSubClusterId(), request.getMethodName(), e);
3356+
Throwable cause = e.getCause();
3357+
if (cause instanceof YarnException) {
3358+
return new SubClusterResult<>(info, null, (YarnException) cause);
3359+
}
3360+
if (cause instanceof IllegalArgumentException) {
3361+
return new SubClusterResult<>(info, null, (IllegalArgumentException) cause);
3362+
}
3363+
if(cause instanceof ForbiddenException) {
3364+
return new SubClusterResult<>(info, null, (ForbiddenException) cause);
3365+
}
33593366
return new SubClusterResult<>(info, null, e);
33603367
}
33613368
});
33623369
}
33633370

3371+
Exception lastException = null;
33643372
for (int i = 0; i < clusterIds.size(); i++) {
33653373
SubClusterInfo subClusterInfo = null;
33663374
try {
@@ -3375,6 +3383,7 @@ private <R> Map<SubClusterInfo, R> invokeConcurrent(Collection<SubClusterInfo> c
33753383

33763384
Exception exception = result.getException();
33773385
if (exception != null) {
3386+
lastException = exception;
33783387
throw exception;
33793388
}
33803389
} catch (Throwable e) {
@@ -3390,6 +3399,9 @@ private <R> Map<SubClusterInfo, R> invokeConcurrent(Collection<SubClusterInfo> c
33903399
}
33913400
}
33923401

3402+
if (results.isEmpty() && lastException != null) {
3403+
throw new YarnRuntimeException(lastException.getMessage());
3404+
}
33933405
return results;
33943406
}
33953407

‎hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@
166166
import org.apache.hadoop.yarn.util.resource.Resources;
167167
import org.junit.jupiter.api.AfterEach;
168168
import org.junit.jupiter.api.BeforeEach;
169+
import org.junit.jupiter.api.Order;
169170
import org.junit.jupiter.api.Test;
170171
import org.slf4j.Logger;
171172
import org.slf4j.LoggerFactory;
@@ -1323,6 +1324,7 @@ public void testGetResourceProfile() throws Exception {
13231324
}
13241325

13251326
@Test
1327+
@Order(1)
13261328
public void testGetAttributesToNodes() throws Exception {
13271329
LOG.info("Test FederationClientInterceptor : Get AttributesToNodes request.");
13281330

‎hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/MockDefaultRequestInterceptorREST.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
package org.apache.hadoop.yarn.server.router.webapp;
2020

2121
import java.io.IOException;
22+
import java.io.PrintWriter;
23+
import java.io.StringWriter;
2224
import java.net.ConnectException;
2325
import java.security.Principal;
2426
import java.util.ArrayList;
@@ -1358,16 +1360,19 @@ public Response addToClusterNodeLabels(NodeLabelsInfo newNodeLabels, HttpServlet
13581360
@Override
13591361
public Response removeFromClusterNodeLabels(Set<String> oldNodeLabels, HttpServletRequest hsr)
13601362
throws Exception {
1363+
Response response = Mockito.mock(Response.class);
13611364
// If oldNodeLabels contains ALL, we let all subclusters pass
13621365
if (oldNodeLabels.contains("ALL")) {
13631366
return Response.status(Status.OK).build();
13641367
} else if (oldNodeLabels.contains("A0")) {
13651368
SubClusterId subClusterId = getSubClusterId();
13661369
String id = subClusterId.getId();
13671370
if (StringUtils.contains("A0", id)) {
1368-
return Response.status(Status.OK).build();
1371+
Mockito.when(response.getStatus()).thenReturn(HttpServletResponse.SC_OK);
1372+
return response;
13691373
} else {
1370-
return Response.status(Status.BAD_REQUEST).entity(null).build();
1374+
Mockito.when(response.getStatus()).thenReturn(HttpServletResponse.SC_BAD_REQUEST);
1375+
return response;
13711376
}
13721377
}
13731378
throw new YarnException("removeFromClusterNodeLabels Error");

‎hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/webapp/TestFederationInterceptorREST.java‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
package org.apache.hadoop.yarn.server.router.webapp;
2020

2121
import java.io.IOException;
22+
import java.io.PrintWriter;
23+
import java.io.StringWriter;
2224
import java.security.Principal;
2325
import java.util.ArrayList;
2426
import java.util.List;
@@ -139,12 +141,14 @@
139141
import org.apache.hadoop.yarn.util.Times;
140142
import org.apache.hadoop.yarn.util.YarnVersionInfo;
141143
import org.apache.hadoop.yarn.webapp.BadRequestException;
144+
import org.apache.hadoop.yarn.webapp.NotFoundException;
142145
import org.apache.hadoop.yarn.webapp.dao.ConfInfo;
143146
import org.apache.hadoop.yarn.webapp.dao.QueueConfigInfo;
144147
import org.apache.hadoop.yarn.webapp.dao.SchedConfUpdateInfo;
145148
import org.apache.hadoop.yarn.webapp.util.WebAppUtils;
146149
import org.junit.jupiter.api.AfterEach;
147150
import org.junit.jupiter.api.BeforeEach;
151+
import org.junit.jupiter.api.Order;
148152
import org.junit.jupiter.api.Test;
149153

150154
import static org.apache.hadoop.yarn.conf.YarnConfiguration.RM_DELEGATION_KEY_UPDATE_INTERVAL_DEFAULT;
@@ -162,6 +166,7 @@
162166
import static org.junit.jupiter.api.Assertions.assertFalse;
163167
import static org.junit.jupiter.api.Assertions.assertNotNull;
164168
import static org.junit.jupiter.api.Assertions.assertNull;
169+
import static org.junit.jupiter.api.Assertions.assertThrows;
165170
import static org.junit.jupiter.api.Assertions.assertTrue;
166171
import static org.mockito.Mockito.mock;
167172
import static org.mockito.Mockito.when;
@@ -527,9 +532,11 @@ public void testGetApplicationNotExists() {
527532
@Test
528533
public void testGetApplicationWrongFormat() {
529534

530-
AppInfo response = interceptor.getApp(null, "Application_wrong_id", null);
531-
532-
assertNull(response);
535+
IllegalArgumentException illegalArgumentException =
536+
assertThrows(IllegalArgumentException.class, () -> {
537+
interceptor.getApp(null, "Application_wrong_id", null);
538+
});
539+
assertTrue(illegalArgumentException.getMessage().contains("Invalid ApplicationId prefix: Application_wrong_id."));
533540
}
534541

535542
/**
@@ -601,6 +608,7 @@ public void testUpdateNodeResource() {
601608
* of SubClusterId. SubClusterId in this case is an integer.
602609
*/
603610
@Test
611+
@Order(1)
604612
public void testGetClusterMetrics() {
605613

606614
ClusterMetricsInfo responseGet = interceptor.getClusterMetricsInfo();
@@ -1363,9 +1371,11 @@ public void testDeleteReservation() throws Exception {
13631371
Response delResponse = interceptor.deleteReservation(deleteRequestInfo, null);
13641372
assertNotNull(delResponse);
13651373

1366-
LambdaTestUtils.intercept(Exception.class,
1367-
"reservationId with id: " + reservationId + " not found",
1368-
() -> interceptor.listReservation(QUEUE_DEDICATED_FULL, applyResId, -1, -1, false, null));
1374+
NotFoundException exception = assertThrows(NotFoundException.class, () -> {
1375+
interceptor.listReservation(QUEUE_DEDICATED_FULL, applyResId, -1, -1, false, null);
1376+
});
1377+
String stackTraceAsString = getStackTraceAsString(exception);
1378+
assertTrue(stackTraceAsString.contains("reservationId with id: " + reservationId + " not found"));
13691379
}
13701380

13711381
private Response submitReservation(ReservationId reservationId)
@@ -1455,16 +1465,23 @@ public void testCheckUserAccessToQueue() throws Exception {
14551465
HttpServletRequest mockHsr = mockHttpServletRequestByUserName("non-admin");
14561466
String errorMsg1 = "User=non-admin doesn't haven access to queue=queue " +
14571467
"so it cannot check ACLs for other users.";
1458-
LambdaTestUtils.intercept(YarnRuntimeException.class, errorMsg1,
1459-
() -> interceptor.checkUserAccessToQueue("queue", "jack",
1460-
QueueACL.SUBMIT_APPLICATIONS.name(), mockHsr));
1468+
RuntimeException exception = assertThrows(RuntimeException.class, () -> {
1469+
interceptor.checkUserAccessToQueue("queue", "jack",
1470+
QueueACL.SUBMIT_APPLICATIONS.name(), mockHsr);
1471+
});
1472+
String stackTraceAsString = getStackTraceAsString(exception);
1473+
assertTrue(stackTraceAsString.contains(errorMsg1));
14611474

14621475
// Case 2: request an unknown ACL causes BAD_REQUEST
14631476
HttpServletRequest mockHsr1 = mockHttpServletRequestByUserName("admin");
14641477
String errorMsg2 = "Specified queueAclType=XYZ_ACL is not a valid type, " +
14651478
"valid queue acl types={SUBMIT_APPLICATIONS/ADMINISTER_QUEUE}";
1466-
LambdaTestUtils.intercept(YarnRuntimeException.class, errorMsg2,
1467-
() -> interceptor.checkUserAccessToQueue("queue", "jack", "XYZ_ACL", mockHsr1));
1479+
RuntimeException exception2 = assertThrows(RuntimeException.class, () -> {
1480+
interceptor.checkUserAccessToQueue("queue", "jack",
1481+
"XYZ_ACL", mockHsr1);
1482+
});
1483+
String stackTraceAsString2 = getStackTraceAsString(exception2);
1484+
assertTrue(stackTraceAsString2.contains(errorMsg2));
14681485

14691486
// We design a test, admin user has ADMINISTER_QUEUE, SUBMIT_APPLICATIONS permissions,
14701487
// yarn user has SUBMIT_APPLICATIONS permissions, other users have no permissions
@@ -1720,8 +1737,11 @@ public void testPostDelegationTokenExpirationError() throws Exception {
17201737

17211738
// If we don't set the header.
17221739
String errorMsg = "Header 'Hadoop-YARN-RM-Delegation-Token' containing encoded token not found";
1723-
LambdaTestUtils.intercept(BadRequestException.class, errorMsg,
1724-
() -> interceptor.postDelegationTokenExpiration(request));
1740+
BadRequestException badRequestException = assertThrows(BadRequestException.class, () -> {
1741+
interceptor.postDelegationTokenExpiration(request);
1742+
});
1743+
String stackTraceAsString = getStackTraceAsString(badRequestException);
1744+
assertTrue(stackTraceAsString.contains(errorMsg));
17251745
}
17261746

17271747
@Test
@@ -2272,4 +2292,11 @@ public void testGetClusterInfo() {
22722292
clusterInfo.getHAZookeeperConnectionState());
22732293
}
22742294
}
2295+
2296+
private String getStackTraceAsString(Exception e) {
2297+
StringWriter sw = new StringWriter();
2298+
PrintWriter pw = new PrintWriter(sw);
2299+
e.printStackTrace(pw);
2300+
return sw.toString();
2301+
}
22752302
}

0 commit comments

Comments
 (0)