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 e7979b8536a..7b8cf602008 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 @@ -231,30 +231,40 @@ public Response deleteService(@Context HttpServletRequest request, e.getCause().getMessage()); } catch (YarnException | FileNotFoundException e) { return formatResponse(Status.NOT_FOUND, e.getMessage()); - } catch (IOException | InterruptedException e) { + } catch (Exception e) { LOG.error("Fail to stop service: {}", e); return formatResponse(Status.INTERNAL_SERVER_ERROR, e.getMessage()); } } private Response stopService(String appName, boolean destroy, - final UserGroupInformation ugi) throws IOException, - InterruptedException, YarnException, FileNotFoundException { + final UserGroupInformation ugi) throws Exception { int result = ugi.doAs(new PrivilegedExceptionAction() { @Override - public Integer run() throws IOException, YarnException, - FileNotFoundException { + public Integer run() throws Exception { int result = 0; ServiceClient sc = getServiceClient(); sc.init(YARN_CONFIG); sc.start(); - result = sc.actionStop(appName, destroy); - if (result == EXIT_SUCCESS) { - LOG.info("Successfully stopped service {}", appName); + Exception stopException = null; + try { + result = sc.actionStop(appName, destroy); + if (result == EXIT_SUCCESS) { + LOG.info("Successfully stopped service {}", appName); + } + } catch (Exception e) { + LOG.info("Got exception stopping service", e); + stopException = e; } if (destroy) { result = sc.actionDestroy(appName); - LOG.info("Successfully deleted service {}", appName); + if (result == EXIT_SUCCESS) { + LOG.info("Successfully deleted service {}", appName); + } + } else { + if (stopException != null) { + throw stopException; + } } sc.close(); return result; @@ -388,7 +398,7 @@ public Response updateService(@Context HttpServletRequest request, String message = "Service is not found in hdfs: " + appName; LOG.error(message, e); return formatResponse(Status.NOT_FOUND, e.getMessage()); - } catch (IOException | InterruptedException e) { + } catch (Exception e) { String message = "Error while performing operation for app: " + appName; LOG.error(message, 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 5d959da3779..5f3e7aa95ec 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 @@ -103,6 +103,8 @@ public int actionDestroy(String serviceName) { } if (serviceName.equals("jenkins")) { return EXIT_SUCCESS; + } if (serviceName.equals("jenkins-already-stopped")) { + return EXIT_SUCCESS; } else { throw new IllegalArgumentException(); } 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 7db6be24cb2..e866739007b 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 @@ -162,6 +162,14 @@ public void testGoodDeleteService() { } @Test + public void testDeleteStoppedService() { + final Response actual = apiServer.deleteService(request, + "jenkins-already-stopped"); + assertEquals("Delete service is ", + Response.status(Status.OK).build().getStatus(), actual.getStatus()); + } + + @Test public void testDecreaseContainerAndStop() { Service service = new Service(); service.setState(ServiceState.STOPPED); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java index 5731e11a197..2454e6f2dd3 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java @@ -326,8 +326,14 @@ private long parseNumberOfContainers(Component component, String newNumber) { .save(fs.getFileSystem(), ServiceApiUtil.getServiceJsonPath(fs, serviceName), persistedService, true); + ApplicationId appId = getAppId(serviceName); + if (appId == null) { + String message = "Application ID doesn't exist for " + serviceName; + LOG.error(message); + throw new YarnException(message); + } ApplicationReport appReport = - yarnClient.getApplicationReport(getAppId(serviceName)); + yarnClient.getApplicationReport(appId); if (appReport.getYarnApplicationState() != RUNNING) { String message = serviceName + " is at " + appReport.getYarnApplicationState() @@ -358,10 +364,16 @@ public int actionStop(String serviceName, boolean waitForAppStopped) throws YarnException, IOException { ServiceApiUtil.validateNameFormat(serviceName, getConfig()); ApplicationId currentAppId = getAppId(serviceName); + if (currentAppId == null) { + LOG.info("Application ID doesn't exist for service {}", serviceName); + cleanUpRegistry(serviceName); + return EXIT_COMMAND_ARGUMENT_ERROR; + } ApplicationReport report = yarnClient.getApplicationReport(currentAppId); if (terminatedStates.contains(report.getYarnApplicationState())) { LOG.info("Service {} is already in a terminated state {}", serviceName, report.getYarnApplicationState()); + cleanUpRegistry(serviceName); return EXIT_COMMAND_ARGUMENT_ERROR; } if (preRunningStates.contains(report.getYarnApplicationState())) { @@ -369,6 +381,7 @@ public int actionStop(String serviceName, boolean waitForAppStopped) + ", forcefully killed by user!"; yarnClient.killApplication(currentAppId, msg); LOG.info(msg); + cleanUpRegistry(serviceName); return EXIT_SUCCESS; } if (StringUtils.isEmpty(report.getHost())) { @@ -388,10 +401,12 @@ public int actionStop(String serviceName, boolean waitForAppStopped) yarnClient.killApplication(currentAppId, serviceName + " is forcefully killed by user!"); LOG.info("Forcefully kill the service: " + serviceName); + cleanUpRegistry(serviceName); return EXIT_SUCCESS; } if (!waitForAppStopped) { + cleanUpRegistry(serviceName); return EXIT_SUCCESS; } // Wait until the app is killed. @@ -421,6 +436,7 @@ public int actionStop(String serviceName, boolean waitForAppStopped) + e.getMessage() + ", forcefully kill the app."); yarnClient.killApplication(currentAppId, "Forcefully kill the app"); } + cleanUpRegistry(serviceName); return EXIT_SUCCESS; } @@ -451,9 +467,24 @@ public int actionDestroy(String serviceName) throws YarnException, } try { deleteZKNode(serviceName); + // don't set destroySucceed to false if no ZK node exists because not + // all services use a ZK node } catch (Exception e) { throw new IOException("Could not delete zk node for " + serviceName, e); } + if (!cleanUpRegistry(serviceName)) { + destroySucceed = false; + } + if (destroySucceed) { + LOG.info("Successfully destroyed service {}", serviceName); + return EXIT_SUCCESS; + } else { + LOG.error("Error on destroy '" + serviceName + "': not found."); + return -1; + } + } + + private boolean cleanUpRegistry(String serviceName) throws SliderException { String registryPath = ServiceRegistryUtils.registryPathForInstance(serviceName); try { @@ -463,18 +494,13 @@ public int actionDestroy(String serviceName) throws YarnException, LOG.info( "Service '" + serviceName + "' doesn't exist at ZK registry path: " + registryPath); - destroySucceed = false; + // not counted as a failure if the registry entries don't exist } } catch (IOException e) { LOG.warn("Error deleting registry entry {}", registryPath, e); + return false; } - if (destroySucceed) { - LOG.info("Successfully destroyed service {}", serviceName); - return EXIT_SUCCESS; - } else { - LOG.error("Error on destroy '" + serviceName + "': not found."); - return -1; - } + return true; } private synchronized RegistryOperations getRegistryClient() @@ -489,17 +515,25 @@ private synchronized RegistryOperations getRegistryClient() return registryClient; } - private boolean deleteZKNode(String clusterName) throws Exception { + /** + * Delete service's ZK node. This is a different node from the service's + * registry entry and is set aside for the service to use for its own ZK data. + * + * @param serviceName service name + * @return true if the node was deleted, false if the node doesn't exist + * @throws Exception if the node couldn't be deleted + */ + private boolean deleteZKNode(String serviceName) throws Exception { CuratorFramework curatorFramework = getCuratorClient(); String user = RegistryUtils.currentUser(); - String zkPath = ServiceRegistryUtils.mkServiceHomePath(user, clusterName); + String zkPath = ServiceRegistryUtils.mkServiceHomePath(user, serviceName); if (curatorFramework.checkExists().forPath(zkPath) != null) { curatorFramework.delete().deletingChildrenIfNeeded().forPath(zkPath); LOG.info("Deleted zookeeper path: " + zkPath); return true; } else { LOG.info( - "Service '" + clusterName + "' doesn't exist at ZK path: " + zkPath); + "Service '" + serviceName + "' doesn't exist at ZK path: " + zkPath); return false; } } @@ -891,6 +925,9 @@ private void addKeytabResourceIfSecure(SliderFileSystem fileSystem, public String updateLifetime(String serviceName, long lifetime) throws YarnException, IOException { ApplicationId currentAppId = getAppId(serviceName); + if (currentAppId == null) { + throw new YarnException("Application ID not found for " + serviceName); + } ApplicationReport report = yarnClient.getApplicationReport(currentAppId); if (report == null) { throw new YarnException("Service not found for " + serviceName); @@ -936,8 +973,8 @@ public String getStatusString(String appIdOrName) throws IOException, YarnException { try { // try parsing appIdOrName, if it succeeds, it means it's appId - ApplicationId.fromString(appIdOrName); - return getStatusByAppId(appIdOrName); + ApplicationId appId = ApplicationId.fromString(appIdOrName); + return getStatusByAppId(appId); } catch (IllegalArgumentException e) { // not appId format, it could be appName. Service status = getStatus(appIdOrName); @@ -945,10 +982,10 @@ public String getStatusString(String appIdOrName) } } - private String getStatusByAppId(String appId) + private String getStatusByAppId(ApplicationId appId) throws IOException, YarnException { ApplicationReport appReport = - yarnClient.getApplicationReport(ApplicationId.fromString(appId)); + yarnClient.getApplicationReport(appId); if (appReport.getYarnApplicationState() != RUNNING) { return ""; @@ -965,10 +1002,14 @@ private String getStatusByAppId(String appId) public Service getStatus(String serviceName) throws IOException, YarnException { ServiceApiUtil.validateNameFormat(serviceName, getConfig()); - ApplicationId currentAppId = getAppId(serviceName); - ApplicationReport appReport = yarnClient.getApplicationReport(currentAppId); Service appSpec = new Service(); appSpec.setName(serviceName); + ApplicationId currentAppId = getAppId(serviceName); + if (currentAppId == null) { + LOG.info("Service {} does not have an application ID", serviceName); + return appSpec; + } + ApplicationReport appReport = yarnClient.getApplicationReport(currentAppId); appSpec.setState(convertState(appReport.getYarnApplicationState())); ApplicationTimeout lifetime = appReport.getApplicationTimeouts().get(ApplicationTimeoutType.LIFETIME); @@ -1084,7 +1125,11 @@ public synchronized ApplicationId getAppId(String serviceName) throw new YarnException("Service " + serviceName + " doesn't exist on hdfs. Please check if the app exists in RM"); } - ApplicationId currentAppId = ApplicationId.fromString(persistedService.getId()); + if (persistedService.getId() == null) { + return null; + } + ApplicationId currentAppId = ApplicationId.fromString(persistedService + .getId()); cachedAppInfo.put(serviceName, new AppInfo(currentAppId, persistedService .getKerberosPrincipal().getPrincipalName())); return currentAppId; diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestYarnNativeServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestYarnNativeServices.java index 091e624d852..bc54183b4bf 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestYarnNativeServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestYarnNativeServices.java @@ -52,6 +52,7 @@ import static org.apache.hadoop.yarn.api.records.YarnApplicationState.FINISHED; import static org.apache.hadoop.yarn.service.conf.YarnServiceConf.YARN_SERVICE_BASE_PATH; +import static org.apache.hadoop.yarn.service.exceptions.LauncherExitCodes.EXIT_COMMAND_ARGUMENT_ERROR; /** * End to end tests to test deploying services with MiniYarnCluster and a in-JVM @@ -135,6 +136,19 @@ public void testCreateFlexStopDestroyService() throws Exception { Assert.assertEquals(-1, client.actionDestroy(exampleApp.getName())); } + // Save a service without starting it and ensure that stop does not NPE and + // that service can be successfully destroyed + @Test (timeout = 200000) + public void testStopDestroySavedService() throws Exception { + setupInternal(NUM_NMS); + ServiceClient client = createClient(); + Service exampleApp = createExampleApplication(); + client.actionBuild(exampleApp); + Assert.assertEquals(EXIT_COMMAND_ARGUMENT_ERROR, client.actionStop( + exampleApp.getName())); + Assert.assertEquals(0, client.actionDestroy(exampleApp.getName())); + } + // Create compa with 2 containers // Create compb with 2 containers which depends on compa // Check containers for compa started before containers for compb