From b09af111179304fee7d64aa1acb561bccffe9a48 Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Thu, 3 May 2018 13:27:07 -0400 Subject: [PATCH] YARN-7961. Improve status message for YARN service. Contributed by Gour Saha (cherry picked from commit 7fe3214d4bb810c0da18dd936875b4e2588ba518) --- .../yarn/service/client/ApiServiceClient.java | 7 +++++++ .../hadoop/yarn/service/webapp/ApiServer.java | 10 +++++++--- .../yarn/service/ServiceClientTest.java | 7 ++++--- .../hadoop/yarn/service/TestApiServer.java | 15 ++++++++++++++- .../service/client/TestApiServiceClient.java | 19 ++++++++++++++++++- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/client/ApiServiceClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/client/ApiServiceClient.java index cdba555c42e..757e6646f8f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/client/ApiServiceClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/client/ApiServiceClient.java @@ -479,6 +479,13 @@ public class ApiServiceClient extends AppAdminClient { try { ClientResponse response = getApiClient(getServicePath(appName)) .get(ClientResponse.class); + if (response.getStatus() == 404) { + StringBuilder sb = new StringBuilder(); + sb.append(" Service "); + sb.append(appName); + sb.append(" not found"); + return sb.toString(); + } if (response.getStatus() != 200) { StringBuilder sb = new StringBuilder(); sb.append(appName); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/webapp/ApiServer.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/webapp/ApiServer.java index 9a30fcfb131..8c7c0ee6aa9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/webapp/ApiServer.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/main/java/org/apache/hadoop/yarn/service/webapp/ApiServer.java @@ -186,7 +186,7 @@ public class ApiServer { ServiceStatus serviceStatus = new ServiceStatus(); try { if (appName == null) { - throw new IllegalArgumentException("Service name can not be null."); + throw new IllegalArgumentException("Service name cannot be null."); } UserGroupInformation ugi = getProxyUser(request); LOG.info("GET: getService for appName = {} user = {}", appName, ugi); @@ -194,12 +194,16 @@ public class ApiServer { return Response.ok(app).build(); } catch (AccessControlException e) { return formatResponse(Status.FORBIDDEN, e.getMessage()); - } catch (IllegalArgumentException | - FileNotFoundException e) { + } catch (IllegalArgumentException e) { serviceStatus.setDiagnostics(e.getMessage()); serviceStatus.setCode(ERROR_CODE_APP_NAME_INVALID); return Response.status(Status.NOT_FOUND).entity(serviceStatus) .build(); + } catch (FileNotFoundException e) { + serviceStatus.setDiagnostics("Service " + appName + " not found"); + serviceStatus.setCode(ERROR_CODE_APP_NAME_INVALID); + return Response.status(Status.NOT_FOUND).entity(serviceStatus) + .build(); } catch (IOException | InterruptedException e) { LOG.error("Get service failed: {}", e); return formatResponse(Status.INTERNAL_SERVER_ERROR, e.getMessage()); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/ServiceClientTest.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/ServiceClientTest.java index cff3e397c0a..73a322cdc6b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/ServiceClientTest.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/ServiceClientTest.java @@ -31,6 +31,7 @@ import org.apache.hadoop.yarn.service.client.ServiceClient; import org.apache.hadoop.yarn.service.utils.ServiceApiUtil; import org.apache.hadoop.yarn.service.utils.SliderFileSystem; +import java.io.FileNotFoundException; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -81,11 +82,11 @@ public class ServiceClientTest extends ServiceClient { } @Override - public Service getStatus(String appName) { - if (appName != null && appName.equals("jenkins")) { + public Service getStatus(String appName) throws FileNotFoundException { + if ("jenkins".equals(appName)) { return goodServiceStatus; } else { - throw new IllegalArgumentException(); + throw new FileNotFoundException("Service " + appName + " not found"); } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/TestApiServer.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/TestApiServer.java index 85c3cd410cd..38aeb59c317 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/TestApiServer.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/TestApiServer.java @@ -41,6 +41,7 @@ import org.apache.hadoop.yarn.service.api.records.Resource; import org.apache.hadoop.yarn.service.api.records.Service; import org.apache.hadoop.yarn.service.api.records.ServiceState; import org.apache.hadoop.yarn.service.api.records.ServiceStatus; +import org.apache.hadoop.yarn.service.conf.RestApiConstants; import org.apache.hadoop.yarn.service.webapp.ApiServer; import org.junit.After; import org.junit.Before; @@ -151,10 +152,17 @@ public class TestApiServer { @Test public void testBadGetService() { - final Response actual = apiServer.getService(request, "no-jenkins"); + final String serviceName = "nonexistent-jenkins"; + final Response actual = apiServer.getService(request, serviceName); assertEquals("Get service is ", Response.status(Status.NOT_FOUND).build().getStatus(), actual.getStatus()); + ServiceStatus serviceStatus = (ServiceStatus) actual.getEntity(); + assertEquals("Response code don't match", + RestApiConstants.ERROR_CODE_APP_NAME_INVALID, serviceStatus.getCode()); + assertEquals("Response diagnostics don't match", + "Service " + serviceName + " not found", + serviceStatus.getDiagnostics()); } @Test @@ -163,6 +171,11 @@ public class TestApiServer { assertEquals("Get service is ", Response.status(Status.NOT_FOUND).build().getStatus(), actual.getStatus()); + ServiceStatus serviceStatus = (ServiceStatus) actual.getEntity(); + assertEquals("Response code don't match", + RestApiConstants.ERROR_CODE_APP_NAME_INVALID, serviceStatus.getCode()); + assertEquals("Response diagnostics don't match", + "Service name cannot be null.", serviceStatus.getDiagnostics()); } @Test diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/client/TestApiServiceClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/client/TestApiServiceClient.java index a24514468ce..fd315700001 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/client/TestApiServiceClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/java/org/apache/hadoop/yarn/service/client/TestApiServiceClient.java @@ -59,7 +59,12 @@ public class TestApiServiceClient { protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { System.out.println("Get was called"); - resp.setStatus(HttpServletResponse.SC_OK); + if (req.getPathInfo() != null + && req.getPathInfo().contains("nonexistent-app")) { + resp.setStatus(HttpServletResponse.SC_NOT_FOUND); + } else { + resp.setStatus(HttpServletResponse.SC_OK); + } } @Override @@ -139,6 +144,18 @@ public class TestApiServiceClient { } } + @Test + public void testStatus() { + String appName = "nonexistent-app"; + try { + String result = asc.getStatusString(appName); + assertEquals("Status reponse don't match", + " Service " + appName + " not found", result); + } catch (IOException | YarnException e) { + fail(); + } + } + @Test public void testStop() { String appName = "example-app";