From 13922cc32d7fabf819592af271ad67e38aecf98d Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Thu, 22 Feb 2018 16:08:30 -0500 Subject: [PATCH] YARN-7836. Added error check for updating service components. (Contributed by Gour Saha) (Cherry-picked from commit 190969006d4a7f9ef86d67bba472f7dc5642668a) --- .../hadoop/yarn/service/webapp/ApiServer.java | 23 +++++-- .../hadoop/yarn/service/TestApiServer.java | 69 ++++++++++++++++--- 2 files changed, 75 insertions(+), 17 deletions(-) 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 e58938e0195..1528596e5ee 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 @@ -280,14 +280,25 @@ public class ApiServer { @PathParam(COMPONENT_NAME) String componentName, Component component) { try { - UserGroupInformation ugi = getProxyUser(request); + if (component == null) { + throw new YarnException("No component data provided"); + } + if (component.getName() != null + && !component.getName().equals(componentName)) { + String msg = "Component name in the request object (" + + component.getName() + ") does not match that in the URI path (" + + componentName + ")"; + throw new YarnException(msg); + } + if (component.getNumberOfContainers() == null) { + throw new YarnException("No container count provided"); + } if (component.getNumberOfContainers() < 0) { - String message = - "Service = " + appName + ", Component = " + component.getName() - + ": Invalid number of containers specified " + component - .getNumberOfContainers(); + String message = "Invalid number of containers specified " + + component.getNumberOfContainers(); throw new YarnException(message); } + UserGroupInformation ugi = getProxyUser(request); Map original = ugi .doAs(new PrivilegedExceptionAction>() { @Override @@ -296,7 +307,7 @@ public class ApiServer { sc.init(YARN_CONFIG); sc.start(); Map original = sc.flexByRestService(appName, - Collections.singletonMap(component.getName(), + Collections.singletonMap(componentName, component.getNumberOfContainers())); sc.close(); return original; 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 52057dbcff0..4629d28b4f7 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 @@ -17,6 +17,16 @@ package org.apache.hadoop.yarn.service; +import static org.junit.Assert.*; + +import java.util.ArrayList; +import java.util.List; + +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.Path; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.yarn.service.api.records.Artifact; import org.apache.hadoop.yarn.service.api.records.Artifact.TypeEnum; @@ -24,23 +34,13 @@ import org.apache.hadoop.yarn.service.api.records.Component; 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.client.ServiceClient; import org.apache.hadoop.yarn.service.webapp.ApiServer; - -import javax.servlet.http.HttpServletRequest; -import javax.ws.rs.Path; -import javax.ws.rs.core.Response; -import javax.ws.rs.core.Response.Status; - import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; -import java.util.ArrayList; -import java.util.List; - -import static org.junit.Assert.*; - /** * Test case for ApiServer REST API. * @@ -370,4 +370,51 @@ public class TestApiServer { Response.status(Status.BAD_REQUEST) .build().getStatus()); } + + @Test + public void testUpdateComponent() { + Response actual = apiServer.updateComponent(request, "jenkins", + "jenkins-master", null); + ServiceStatus serviceStatus = (ServiceStatus) actual.getEntity(); + assertEquals("Update component should have failed with 400 bad request", + Response.status(Status.BAD_REQUEST).build().getStatus(), + actual.getStatus()); + assertEquals("Update component should have failed with no data error", + "No component data provided", serviceStatus.getDiagnostics()); + + Component comp = new Component(); + actual = apiServer.updateComponent(request, "jenkins", "jenkins-master", + comp); + serviceStatus = (ServiceStatus) actual.getEntity(); + assertEquals("Update component should have failed with 400 bad request", + Response.status(Status.BAD_REQUEST).build().getStatus(), + actual.getStatus()); + assertEquals("Update component should have failed with no count error", + "No container count provided", serviceStatus.getDiagnostics()); + + comp.setNumberOfContainers(-1L); + actual = apiServer.updateComponent(request, "jenkins", "jenkins-master", + comp); + serviceStatus = (ServiceStatus) actual.getEntity(); + assertEquals("Update component should have failed with 400 bad request", + Response.status(Status.BAD_REQUEST).build().getStatus(), + actual.getStatus()); + assertEquals("Update component should have failed with no count error", + "Invalid number of containers specified -1", serviceStatus.getDiagnostics()); + + comp.setName("jenkins-slave"); + comp.setNumberOfContainers(1L); + actual = apiServer.updateComponent(request, "jenkins", "jenkins-master", + comp); + serviceStatus = (ServiceStatus) actual.getEntity(); + assertEquals("Update component should have failed with 400 bad request", + Response.status(Status.BAD_REQUEST).build().getStatus(), + actual.getStatus()); + assertEquals( + "Update component should have failed with component name mismatch " + + "error", + "Component name in the request object (jenkins-slave) does not match " + + "that in the URI path (jenkins-master)", + serviceStatus.getDiagnostics()); + } }