YARN-9074. Consolidate docker removal logic in ContainerCleanup.

           Contributed by Zhaohui Xin

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

@@ -62,16 +62,13 @@ import org.apache.hadoop.yarn.security.ContainerTokenIdentifier;
 import org.apache.hadoop.yarn.server.api.protocolrecords.NMContainerStatus;
 import org.apache.hadoop.yarn.server.nodemanager.ContainerExecutor.ExitCode;
 import org.apache.hadoop.yarn.server.nodemanager.Context;
-import org.apache.hadoop.yarn.server.nodemanager.DeletionService;
 import org.apache.hadoop.yarn.server.nodemanager.NMAuditLogger;
 import org.apache.hadoop.yarn.server.nodemanager.NMAuditLogger.AuditConstants;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServicesEvent;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServicesEventType;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.application.ApplicationContainerFinishedEvent;
-import org.apache.hadoop.yarn.server.nodemanager.containermanager.deletion.task.DockerContainerDeletionTask;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainersLauncherEvent;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainersLauncherEventType;
-import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DockerLinuxContainerRuntime;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.LocalResourceRequest;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ResourceSet;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.event.ContainerLocalizationCleanupEvent;
@@ -1569,12 +1566,6 @@ public class ContainerImpl implements Container {
     	
       // TODO: Add containerWorkDir to the deletion service.
 
-      if (DockerLinuxContainerRuntime.isDockerContainerRequested(
-          container.daemonConf,
-          container.getLaunchContext().getEnvironment())) {
-        removeDockerContainer(container);
-      }
-
       if (clCleanupRequired) {
         container.dispatcher.getEventHandler().handle(
             new ContainersLauncherEvent(container,
@@ -1610,12 +1601,6 @@ public class ContainerImpl implements Container {
       // TODO: Add containerWorkDir to the deletion service.
       // TODO: Add containerOuputDir to the deletion service.
 
-      if (DockerLinuxContainerRuntime.isDockerContainerRequested(
-          container.daemonConf,
-          container.getLaunchContext().getEnvironment())) {
-        removeDockerContainer(container);
-      }
-
       if (clCleanupRequired) {
         container.dispatcher.getEventHandler().handle(
             new ContainersLauncherEvent(container,
@@ -1896,12 +1881,6 @@ public class ContainerImpl implements Container {
         container.addDiagnostics(exitEvent.getDiagnosticInfo() + "\n");
       }
 
-      if (DockerLinuxContainerRuntime.isDockerContainerRequested(
-          container.daemonConf,
-          container.getLaunchContext().getEnvironment())) {
-        removeDockerContainer(container);
-      }
-
       // The process/process-grp is killed. Decrement reference counts and
       // cleanup resources
       container.cleanup();
@@ -2240,14 +2219,6 @@ public class ContainerImpl implements Container {
     return resourceMappings;
   }
 
-  private static void removeDockerContainer(ContainerImpl container) {
-    DeletionService deletionService = container.context.getDeletionService();
-    DockerContainerDeletionTask deletionTask =
-        new DockerContainerDeletionTask(deletionService, container.user,
-            container.getContainerId().toString());
-    deletionService.delete(deletionTask);
-  }
-
   private void storeRetryContext() {
     if (windowRetryContext.getRestartTimes() != null &&
         !windowRetryContext.getRestartTimes().isEmpty()) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerCleanup.java

@@ -29,10 +29,12 @@ import org.apache.hadoop.yarn.conf.YarnConfiguration;
 import org.apache.hadoop.yarn.event.Dispatcher;
 import org.apache.hadoop.yarn.server.nodemanager.ContainerExecutor;
 import org.apache.hadoop.yarn.server.nodemanager.Context;
+import org.apache.hadoop.yarn.server.nodemanager.DeletionService;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Container;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerDiagnosticsUpdateEvent;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerEventType;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerExitEvent;
+import org.apache.hadoop.yarn.server.nodemanager.containermanager.deletion.task.DockerContainerDeletionTask;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DockerLinuxContainerRuntime;
 import org.apache.hadoop.yarn.server.nodemanager.executor.ContainerReapContext;
 import org.apache.hadoop.yarn.server.nodemanager.executor.ContainerSignalContext;
@@ -145,11 +147,12 @@ public class ContainerCleanup implements Runnable {
           // Increasing YarnConfiguration.NM_PROCESS_KILL_WAIT_MS
           // reduces the likelihood of this race condition and process leak.
         }
-        // The Docker container may not have fully started, reap the container.
-        if (DockerLinuxContainerRuntime.isDockerContainerRequested(conf,
-            container.getLaunchContext().getEnvironment())) {
-          reapDockerContainerNoPid(user);
-        }
+      }
+
+      // rm container in docker
+      if (DockerLinuxContainerRuntime.isDockerContainerRequested(conf,
+          container.getLaunchContext().getEnvironment())) {
+        rmDockerContainerDelayed();
       }
     } catch (Exception e) {
       String message =
@@ -181,6 +184,14 @@ public class ContainerCleanup implements Runnable {
     }
   }
 
+  private void rmDockerContainerDelayed() {
+    DeletionService deletionService = context.getDeletionService();
+    DockerContainerDeletionTask deletionTask =
+        new DockerContainerDeletionTask(deletionService, container.getUser(),
+            container.getContainerId().toString());
+    deletionService.delete(deletionTask);
+  }
+
   private void signalProcess(String processId, String user,
       String containerIdStr) throws IOException {
     if (LOG.isDebugEnabled()) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/TestContainer.java

@@ -82,7 +82,6 @@ import org.apache.hadoop.yarn.server.nodemanager.containermanager.AuxServicesEve
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.application.Application;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.application.ApplicationEvent;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.application.ApplicationEventType;
-import org.apache.hadoop.yarn.server.nodemanager.containermanager.deletion.task.DockerContainerDeletionMatcher;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainersLauncher;
 import org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainersLauncherEvent;
@@ -1168,10 +1167,10 @@ public class TestContainer {
 
   private void verifyDockerContainerCleanupCall(WrappedContainer wc)
       throws Exception {
-    DeletionService delService = wc.context.getDeletionService();
-    verify(delService, times(1)).delete(argThat(
-        new DockerContainerDeletionMatcher(delService,
-            wc.c.getContainerId().toString())));
+    // check if containerlauncher cleans up the container launch.
+    verify(wc.launcherBus)
+        .handle(refEq(new ContainersLauncherEvent(wc.c,
+            ContainersLauncherEventType.CLEANUP_CONTAINER), "timestamp"));
   }
 
   // Argument matcher for matching container localization cleanup event.
@@ -1580,8 +1579,10 @@ public class TestContainer {
     public void dockerContainerResourcesCleanup() {
       c.handle(new ContainerEvent(cId,
           ContainerEventType.CONTAINER_RESOURCES_CLEANEDUP));
-      verify(delService, times(1)).delete(argThat(
-          new DockerContainerDeletionMatcher(delService, cId.toString())));
+      // check if containerlauncher cleans up the container launch.
+      verify(this.launcherBus)
+          .handle(refEq(new ContainersLauncherEvent(this.c,
+              ContainersLauncherEventType.CLEANUP_CONTAINER), "timestamp"));
       drainDispatcherEvents();
     }