YARN-7740. Fix logging for destroy yarn service cli when app does not exist and some minor bugs. Contributed by Jian He

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/ServiceScheduler.java

@@ -399,7 +399,7 @@ public class ServiceScheduler extends CompositeService {
       LOG.error("Failed to get user.", e);
     }
     globalTokens
-        .put(SERVICE_ZK_PATH, ServiceRegistryUtils.mkClusterPath(user, app.getName()));
+        .put(SERVICE_ZK_PATH, ServiceRegistryUtils.mkServiceHomePath(user, app.getName()));
 
     globalTokens.put(ServiceApiConstants.USER, user);
     String dnsDomain = getConfig().getTrimmed(KEY_DNS_DOMAIN);

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

@@ -433,6 +433,7 @@ public class ServiceClient extends AppAdminClient implements SliderExitCodes,
     FileSystem fileSystem = fs.getFileSystem();
     // remove from the appId cache
     cachedAppInfo.remove(serviceName);
+    boolean destroySucceed = true;
     if (fileSystem.exists(appDir)) {
       if (fileSystem.delete(appDir, true)) {
         LOG.info("Successfully deleted service dir for " + serviceName + ": "
@@ -443,20 +444,37 @@ public class ServiceClient extends AppAdminClient implements SliderExitCodes,
         LOG.info(message);
         throw new YarnException(message);
       }
+    } else {
+      LOG.info("Service '" + serviceName + "' doesn't exist at hdfs path: "
+          + appDir);
+      destroySucceed = false;
     }
     try {
       deleteZKNode(serviceName);
     } catch (Exception e) {
       throw new IOException("Could not delete zk node for " + serviceName, e);
     }
-    String registryPath = ServiceRegistryUtils.registryPathForInstance(serviceName);
+    String registryPath =
+        ServiceRegistryUtils.registryPathForInstance(serviceName);
     try {
-      getRegistryClient().delete(registryPath, true);
+      if (getRegistryClient().exists(registryPath)) {
+        getRegistryClient().delete(registryPath, true);
+      } else {
+        LOG.info(
+            "Service '" + serviceName + "' doesn't exist at ZK registry path: "
+                + registryPath);
+        destroySucceed = false;
+      }
     } catch (IOException e) {
       LOG.warn("Error deleting registry entry {}", registryPath, e);
     }
-    LOG.info("Destroyed cluster {}", serviceName);
-    return EXIT_SUCCESS;
+    if (destroySucceed) {
+      LOG.info("Successfully destroyed service {}", serviceName);
+      return EXIT_SUCCESS;
+    } else {
+      LOG.error("Error on destroy '" + serviceName + "': not found.");
+      return -1;
+    }
   }
 
   private synchronized RegistryOperations getRegistryClient()
@@ -471,13 +489,18 @@ public class ServiceClient extends AppAdminClient implements SliderExitCodes,
     return registryClient;
   }
 
-  private void deleteZKNode(String clusterName) throws Exception {
+  private boolean deleteZKNode(String clusterName) throws Exception {
     CuratorFramework curatorFramework = getCuratorClient();
     String user = RegistryUtils.currentUser();
-    String zkPath = ServiceRegistryUtils.mkClusterPath(user, clusterName);
+    String zkPath = ServiceRegistryUtils.mkServiceHomePath(user, clusterName);
     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);
+      return false;
     }
   }
 
@@ -908,7 +931,7 @@ public class ServiceClient extends AppAdminClient implements SliderExitCodes,
     } catch (IllegalArgumentException e) {
       // not appId format, it could be appName.
       Service status = getStatus(appIdOrName);
-      return status.toString();
+      return ServiceApiUtil.jsonSerDeser.toJson(status);
     }
   }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/provider/AbstractClientProvider.java

@@ -101,8 +101,8 @@ public abstract class AbstractClientProvider {
         Path p = new Path(file.getSrcFile());
         if (!fs.exists(p)) {
           throw new IllegalArgumentException(
-              "Src_file does not exist for config file: " + file
-                  .getSrcFile());
+              "Specified src_file does not exist on " + fs.getScheme() + ": "
+                  + file.getSrcFile());
         }
       }
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceRegistryUtils.java

@@ -24,18 +24,7 @@ import org.apache.hadoop.yarn.service.conf.YarnServiceConstants;
 
 public class ServiceRegistryUtils {
 
-  /**
-   * Base path for services
-   */
-  public static final String ZK_SERVICES = "services";
-
-  /**
-   * Base path for all Slider references
-   */
-  public static final String ZK_SLIDER = "slider";
-  public static final String ZK_USERS = "users";
-  public static final String SVC_SLIDER = "/" + ZK_SERVICES + "/" + ZK_SLIDER;
-  public static final String SVC_SLIDER_USERS = SVC_SLIDER + "/" + ZK_USERS;
+  public static final String SVC_USERS = "/services/yarn/users";
 
   /**
    * Get the registry path for an instance under the user's home node
@@ -49,23 +38,19 @@ public class ServiceRegistryUtils {
   }
 
   /**
- * Build the path to a cluster; exists once the cluster has come up.
- * Even before that, a ZK watcher could wait for it.
- * @param username user
- * @param clustername name of the cluster
- * @return a strin
+ * Build the path to a service folder
+ * @param username user name
+ * @param serviceName service name
+ * @return the home path to the service
  */
-  public static String mkClusterPath(String username, String clustername) {
-    return mkSliderUserPath(username) + "/" + clustername;
+  public static String mkServiceHomePath(String username, String serviceName) {
+    return mkUserHomePath(username) + "/" + serviceName;
   }
 
   /**
- * Build the path to a cluster; exists once the cluster has come up.
- * Even before that, a ZK watcher could wait for it.
- * @param username user
- * @return a string
- */
-  public static String mkSliderUserPath(String username) {
-    return SVC_SLIDER_USERS + "/" + username;
+   * Build the path to a user home folder;
+   */
+  public static String mkUserHomePath(String username) {
+    return SVC_USERS + "/" + username;
   }
 }

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

@@ -123,10 +123,13 @@ public class TestYarnNativeServices extends ServiceTestUtils {
         report.getFinalApplicationStatus());
 
     LOG.info("Destroy the service");
-    //destroy the service and check the app dir is deleted from fs.
-    client.actionDestroy(exampleApp.getName());
+    // destroy the service and check the app dir is deleted from fs.
+    Assert.assertEquals(0, client.actionDestroy(exampleApp.getName()));
     // check the service dir on hdfs (in this case, local fs) are deleted.
     Assert.assertFalse(getFS().exists(appDir));
+
+    // check that destroying again does not succeed
+    Assert.assertEquals(-1, client.actionDestroy(exampleApp.getName()));
   }
 
   // Create compa with 2 containers

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/ContainerImpl.java

@@ -34,6 +34,7 @@ import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.yarn.api.records.ContainerSubState;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.scheduler.UpdateContainerSchedulerEvent;
 import org.slf4j.Logger;
@@ -834,7 +835,8 @@ public class ContainerImpl implements Container {
       ContainerStatus status = BuilderUtils.newContainerStatus(this.containerId,
           getCurrentState(), diagnostics.toString(), exitCode, getResource(),
           this.containerTokenIdentifier.getExecutionType());
-      status.setIPs(ips == null ? null : Arrays.asList(ips.split(",")));
+      status.setIPs(StringUtils.isEmpty(ips) ? null :
+          Arrays.asList(ips.split(",")));
       status.setHost(host);
       status.setContainerSubState(getContainerSubState());
       return status;