YARN-7836. Added error check for updating service components.

            (Contributed by Gour Saha)

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<String, Long> original = ugi
           .doAs(new PrivilegedExceptionAction<Map<String, Long>>() {
             @Override
@@ -296,7 +307,7 @@ public class ApiServer {
               sc.init(YARN_CONFIG);
               sc.start();
               Map<String, Long> original = sc.flexByRestService(appName,
-                  Collections.singletonMap(component.getName(),
+                  Collections.singletonMap(componentName,
                       component.getNumberOfContainers()));
               sc.close();
               return original;

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());
+  }
 }