From 2e636dd3c497a9f0042642296b127686012de57a Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Mon, 28 Jan 2019 18:05:53 -0500 Subject: [PATCH] YARN-9074. Consolidate docker removal logic in ContainerCleanup. Contributed by Zhaohui Xin --- .../container/ContainerImpl.java | 29 ------------------- .../launcher/ContainerCleanup.java | 21 ++++++++++---- .../container/TestContainer.java | 15 +++++----- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/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 b/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 index cfa62577ff..8aa8d0749e 100644 --- a/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 +++ b/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()) { diff --git a/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 b/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 index 963d28b54f..5800ef5cbf 100644 --- a/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 +++ b/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()) { diff --git a/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 b/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 index 5cabd6d3ec..95ff9684ce 100644 --- a/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 +++ b/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(); }