From 81f1a46b3bb752dfaac4b7c4b9a795cc3d9be62c Mon Sep 17 00:00:00 2001 From: Craig Condit Date: Fri, 27 Jul 2018 18:04:24 -0500 Subject: [PATCH] YARN-8263. DockerClient still touches hadoop.tmp.dir. - Removed unused writeCommandToTempFile() method from DockerClient. - Removed unnecessary constructor from DockerClient. - Removed unused conf parameter from DCE#preparePrivilegedOperation. - Removed unused conf parameter from DCE#executeDockerCommand. --- .../server/nodemanager/LinuxContainerExecutor.java | 2 +- .../linux/runtime/DockerLinuxContainerRuntime.java | 9 ++-- .../linux/runtime/docker/DockerClient.java | 53 ---------------------- .../linux/runtime/docker/DockerCommand.java | 5 +- .../runtime/docker/DockerCommandExecutor.java | 8 ++-- .../linux/runtime/docker/DockerInspectCommand.java | 2 +- .../linux/runtime/docker/DockerRmCommand.java | 2 +- .../linux/runtime/docker/TestDockerClient.java | 2 +- .../runtime/docker/TestDockerCommandExecutor.java | 18 ++++---- 9 files changed, 22 insertions(+), 79 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/LinuxContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java index 4253f2f0d54..9b48c1aa757 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java @@ -949,7 +949,7 @@ public void removeDockerContainer(String containerId) { LOG.info("Removing Docker container : " + containerId); DockerRmCommand dockerRmCommand = new DockerRmCommand(containerId); DockerCommandExecutor.executeDockerCommand(dockerRmCommand, containerId, - null, super.getConf(), privOpExecutor, false, nmContext); + null, privOpExecutor, false, nmContext); } } catch (ContainerExecutionException e) { LOG.warn("Unable to remove docker container: " + containerId); 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/linux/runtime/DockerLinuxContainerRuntime.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java index 88e6c91dac7..ad0fd138e21 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java @@ -298,7 +298,7 @@ public void initialize(Configuration conf, Context nmContext) throws ContainerExecutionException { this.nmContext = nmContext; this.conf = conf; - dockerClient = new DockerClient(conf); + dockerClient = new DockerClient(); allowedNetworks.clear(); defaultROMounts.clear(); defaultRWMounts.clear(); @@ -1225,7 +1225,7 @@ private void handleContainerStop(String containerId, Map env) DockerStopCommand dockerStopCommand = new DockerStopCommand( containerId).setGracePeriod(dockerStopGracePeriod); DockerCommandExecutor.executeDockerCommand(dockerStopCommand, containerId, - env, conf, privilegedOperationExecutor, false, nmContext); + env, privilegedOperationExecutor, false, nmContext); } else { if (LOG.isDebugEnabled()) { LOG.debug( @@ -1253,8 +1253,7 @@ private void handleContainerKill(ContainerRuntimeContext ctx, DockerKillCommand dockerKillCommand = new DockerKillCommand(containerId).setSignal(signal.name()); DockerCommandExecutor.executeDockerCommand(dockerKillCommand, - containerId, env, conf, privilegedOperationExecutor, false, - nmContext); + containerId, env, privilegedOperationExecutor, false, nmContext); } else { LOG.debug( "Container status is {}, skipping kill - {}", @@ -1297,7 +1296,7 @@ private void handleContainerRemove(String containerId, if (DockerCommandExecutor.isRemovable(containerStatus)) { DockerRmCommand dockerRmCommand = new DockerRmCommand(containerId); DockerCommandExecutor.executeDockerCommand(dockerRmCommand, containerId, - env, conf, privilegedOperationExecutor, false, nmContext); + env, privilegedOperationExecutor, false, nmContext); } } } 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/linux/runtime/docker/DockerClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java index fca707ce31c..3a516c46efe 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerClient.java @@ -22,7 +22,6 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.yarn.api.records.ApplicationId; @@ -50,58 +49,6 @@ private static final String TMP_FILE_PREFIX = "docker."; private static final String TMP_FILE_SUFFIX = ".cmd"; private static final String TMP_ENV_FILE_SUFFIX = ".env"; - private final String tmpDirPath; - - public DockerClient(Configuration conf) throws ContainerExecutionException { - - String tmpDirBase = conf.get("hadoop.tmp.dir"); - if (tmpDirBase == null) { - throw new ContainerExecutionException("hadoop.tmp.dir not set!"); - } - tmpDirPath = tmpDirBase + "/nm-docker-cmds"; - - File tmpDir = new File(tmpDirPath); - if (!(tmpDir.exists() || tmpDir.mkdirs())) { - LOG.warn("Unable to create directory: " + tmpDirPath); - throw new ContainerExecutionException("Unable to create directory: " + - tmpDirPath); - } - } - - public String writeCommandToTempFile(DockerCommand cmd, String filePrefix) - throws ContainerExecutionException { - try { - File dockerCommandFile = File.createTempFile(TMP_FILE_PREFIX + filePrefix, - TMP_FILE_SUFFIX, new - File(tmpDirPath)); - try ( - Writer writer = new OutputStreamWriter( - new FileOutputStream(dockerCommandFile), "UTF-8"); - PrintWriter printWriter = new PrintWriter(writer); - ) { - printWriter.println("[docker-command-execution]"); - for (Map.Entry> entry : - cmd.getDockerCommandWithArguments().entrySet()) { - if (entry.getKey().contains("=")) { - throw new ContainerExecutionException( - "'=' found in entry for docker command file, key = " + entry - .getKey() + "; value = " + entry.getValue()); - } - if (entry.getValue().contains("\n")) { - throw new ContainerExecutionException( - "'\\n' found in entry for docker command file, key = " + entry - .getKey() + "; value = " + entry.getValue()); - } - printWriter.println(" " + entry.getKey() + "=" + StringUtils - .join(",", entry.getValue())); - } - return dockerCommandFile.getAbsolutePath(); - } - } catch (IOException e) { - LOG.warn("Unable to write docker command to temporary file!"); - throw new ContainerExecutionException(e); - } - } private String writeEnvFile(DockerRunCommand cmd, String filePrefix, File cmdDir) throws IOException { 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/linux/runtime/docker/DockerCommand.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerCommand.java index 366457d11e4..37969817b05 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerCommand.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerCommand.java @@ -117,16 +117,15 @@ public void setClientConfigDir(String clientConfigDir) { * @param dockerCommand Specific command to be run by docker. * @param containerName * @param env - * @param conf * @param nmContext * @return Returns the PrivilegedOperation object to be used. * @throws ContainerExecutionException */ public PrivilegedOperation preparePrivilegedOperation( DockerCommand dockerCommand, String containerName, Map env, Configuration conf, Context nmContext) + String> env, Context nmContext) throws ContainerExecutionException { - DockerClient dockerClient = new DockerClient(conf); + DockerClient dockerClient = new DockerClient(); String commandFile = dockerClient.writeCommandToTempFile(dockerCommand, ContainerId.fromString(containerName), 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/linux/runtime/docker/DockerCommandExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerCommandExecutor.java index 8a4888cb2cd..6828766faa8 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerCommandExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerCommandExecutor.java @@ -68,19 +68,18 @@ private DockerCommandExecutor() { * @param dockerCommand the docker command to run. * @param containerId the id of the container. * @param env environment for the container. - * @param conf the hadoop configuration. * @param privilegedOperationExecutor the privileged operations executor. * @param disableFailureLogging disable logging for known rc failures. * @return the output of the operation. * @throws ContainerExecutionException if the operation fails. */ public static String executeDockerCommand(DockerCommand dockerCommand, - String containerId, Map env, Configuration conf, + String containerId, Map env, PrivilegedOperationExecutor privilegedOperationExecutor, boolean disableFailureLogging, Context nmContext) throws ContainerExecutionException { PrivilegedOperation dockerOp = dockerCommand.preparePrivilegedOperation( - dockerCommand, containerId, env, conf, nmContext); + dockerCommand, containerId, env, nmContext); if (disableFailureLogging) { dockerOp.disableFailureLogging(); @@ -184,8 +183,7 @@ private static String executeStatusCommand(String containerId, new DockerInspectCommand(containerId).getContainerStatus(); try { return DockerCommandExecutor.executeDockerCommand(dockerInspectCommand, - containerId, null, conf, privilegedOperationExecutor, true, - nmContext); + containerId, null, privilegedOperationExecutor, true, nmContext); } catch (ContainerExecutionException e) { throw new ContainerExecutionException(e); } 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/linux/runtime/docker/DockerInspectCommand.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerInspectCommand.java index 3ed9c185243..310fc977b98 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerInspectCommand.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerInspectCommand.java @@ -58,7 +58,7 @@ public DockerInspectCommand getIpAndHost() { @Override public PrivilegedOperation preparePrivilegedOperation( DockerCommand dockerCommand, String containerName, Map env, Configuration conf, Context nmContext) { + String> env, Context nmContext) { PrivilegedOperation dockerOp = new PrivilegedOperation( PrivilegedOperation.OperationType.INSPECT_DOCKER_CONTAINER); dockerOp.appendArgs(commandArguments, containerName); 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/linux/runtime/docker/DockerRmCommand.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerRmCommand.java index 3a02982d0d0..cfcf13df16f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerRmCommand.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/DockerRmCommand.java @@ -37,7 +37,7 @@ public DockerRmCommand(String containerName) { @Override public PrivilegedOperation preparePrivilegedOperation( DockerCommand dockerCommand, String containerName, Map env, Configuration conf, Context nmContext) { + String> env, Context nmContext) { PrivilegedOperation dockerOp = new PrivilegedOperation( PrivilegedOperation.OperationType.REMOVE_DOCKER_CONTAINER); dockerOp.appendArgs(containerName); 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/linux/runtime/docker/TestDockerClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java index efd7db5f1cf..31645bcde44 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerClient.java @@ -68,7 +68,7 @@ public void testWriteCommandToTempFile() throws Exception { doReturn(conf).when(mockContext).getConf(); doReturn(dirsHandler).when(mockContext).getLocalDirsHandler(); - DockerClient dockerClient = new DockerClient(conf); + DockerClient dockerClient = new DockerClient(); dirsHandler.init(conf); dirsHandler.start(); String tmpPath = dockerClient.writeCommandToTempFile(dockerCmd, cid, 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/linux/runtime/docker/TestDockerCommandExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerCommandExecutor.java index 50d00bb518c..e040e97bbde 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerCommandExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/docker/TestDockerCommandExecutor.java @@ -138,7 +138,7 @@ public void testExecuteDockerCommand() throws Exception { DockerStopCommand dockerStopCommand = new DockerStopCommand(MOCK_CONTAINER_ID); DockerCommandExecutor.executeDockerCommand(dockerStopCommand, - cId.toString(), env, configuration, mockExecutor, false, nmContext); + cId.toString(), env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); assertEquals(1, ops.size()); @@ -150,7 +150,7 @@ public void testExecuteDockerCommand() throws Exception { public void testExecuteDockerRm() throws Exception { DockerRmCommand dockerCommand = new DockerRmCommand(MOCK_CONTAINER_ID); DockerCommandExecutor.executeDockerCommand(dockerCommand, MOCK_CONTAINER_ID, - env, configuration, mockExecutor, false, nmContext); + env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); PrivilegedOperation privOp = ops.get(0); @@ -167,7 +167,7 @@ public void testExecuteDockerRm() throws Exception { public void testExecuteDockerStop() throws Exception { DockerStopCommand dockerCommand = new DockerStopCommand(MOCK_CONTAINER_ID); DockerCommandExecutor.executeDockerCommand(dockerCommand, MOCK_CONTAINER_ID, - env, configuration, mockExecutor, false, nmContext); + env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); List dockerCommands = getValidatedDockerCommands(ops); @@ -185,7 +185,7 @@ public void testExecuteDockerInspectStatus() throws Exception { DockerInspectCommand dockerCommand = new DockerInspectCommand(MOCK_CONTAINER_ID).getContainerStatus(); DockerCommandExecutor.executeDockerCommand(dockerCommand, MOCK_CONTAINER_ID, - env, configuration, mockExecutor, false, nmContext); + env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); PrivilegedOperation privOp = ops.get(0); @@ -204,7 +204,7 @@ public void testExecuteDockerPull() throws Exception { DockerPullCommand dockerCommand = new DockerPullCommand(MOCK_IMAGE_NAME); DockerCommandExecutor.executeDockerCommand(dockerCommand, MOCK_CONTAINER_ID, - env, configuration, mockExecutor, false, nmContext); + env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); List dockerCommands = getValidatedDockerCommands(ops); @@ -222,7 +222,7 @@ public void testExecuteDockerLoad() throws Exception { DockerLoadCommand dockerCommand = new DockerLoadCommand(MOCK_LOCAL_IMAGE_NAME); DockerCommandExecutor.executeDockerCommand(dockerCommand, MOCK_CONTAINER_ID, - env, configuration, mockExecutor, false, nmContext); + env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); List dockerCommands = getValidatedDockerCommands(ops); @@ -254,7 +254,7 @@ public void testExecuteDockerKillSIGQUIT() throws Exception { new DockerKillCommand(MOCK_CONTAINER_ID) .setSignal(ContainerExecutor.Signal.QUIT.name()); DockerCommandExecutor.executeDockerCommand(dockerKillCommand, - MOCK_CONTAINER_ID, env, configuration, mockExecutor, false, nmContext); + MOCK_CONTAINER_ID, env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); List dockerCommands = getValidatedDockerCommands(ops); @@ -275,7 +275,7 @@ public void testExecuteDockerKillSIGKILL() throws Exception { new DockerKillCommand(MOCK_CONTAINER_ID) .setSignal(ContainerExecutor.Signal.KILL.name()); DockerCommandExecutor.executeDockerCommand(dockerKillCommand, - MOCK_CONTAINER_ID, env, configuration, mockExecutor, false, nmContext); + MOCK_CONTAINER_ID, env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); List dockerCommands = getValidatedDockerCommands(ops); @@ -296,7 +296,7 @@ public void testExecuteDockerKillSIGTERM() throws Exception { new DockerKillCommand(MOCK_CONTAINER_ID) .setSignal(ContainerExecutor.Signal.TERM.name()); DockerCommandExecutor.executeDockerCommand(dockerKillCommand, - MOCK_CONTAINER_ID, env, configuration, mockExecutor, false, nmContext); + MOCK_CONTAINER_ID, env, mockExecutor, false, nmContext); List ops = MockPrivilegedOperationCaptor .capturePrivilegedOperations(mockExecutor, 1, true); List dockerCommands = getValidatedDockerCommands(ops); -- 2.15.2 (Apple Git-101.1)