diff options
author | valerijf <valerijf@yahoo-inc.com> | 2017-08-08 09:46:37 +0200 |
---|---|---|
committer | valerijf <valerijf@yahoo-inc.com> | 2017-08-08 09:46:37 +0200 |
commit | b6c676f7ef9ac11088bc2559e7c17aed0163e67c (patch) | |
tree | 282df4d28eb7f6d4354de30c541ba54bd49be8ac /node-admin/src | |
parent | c983e3bb1886d1eea4a2c246b0d81221ef97b84c (diff) |
Use ProcessExecutor for Docker container ACL setup instead of docker.exec()
Diffstat (limited to 'node-admin/src')
4 files changed, 32 insertions, 18 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java index 4409fa63133..9e6f7a25ed8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java @@ -2,10 +2,12 @@ package com.yahoo.vespa.hosted.node.admin.docker; import static com.yahoo.vespa.defaults.Defaults.getDefaults; + +import com.yahoo.collections.Pair; +import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.Docker; -import com.yahoo.vespa.hosted.dockerapi.DockerExecTimeoutException; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.dockerapi.DockerImpl; import com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator; @@ -83,10 +85,12 @@ public class DockerOperationsImpl implements DockerOperations { private final Docker docker; private final Environment environment; + private final ProcessExecuter processExecuter; - public DockerOperationsImpl(Docker docker, Environment environment) { + public DockerOperationsImpl(Docker docker, Environment environment, ProcessExecuter processExecuter) { this.docker = docker; this.environment = environment; + this.processExecuter = processExecuter; } // Returns empty if vespa version cannot be parsed. @@ -267,22 +271,23 @@ public class DockerOperationsImpl implements DockerOperations { containerName.asString())); final String[] wrappedCommand = Stream.concat( - Stream.of("nsenter", String.format("--net=/host/proc/%d/ns/net", containerPid), "--"), + Stream.of("sudo", "nsenter", String.format("--net=/host/proc/%d/ns/net", containerPid), "--"), Stream.of(command)) .toArray(String[]::new); try { - ProcessResult result = docker.executeInContainerAsRoot(new ContainerName("node-admin"), 60L, wrappedCommand); - if (! result.isSuccess()) { - String msg = String.format("Failed to execute %s in network namespace for %s (PID = %d)", - Arrays.toString(wrappedCommand), containerName.asString(), containerPid); + Pair<Integer, String> result = processExecuter.exec(wrappedCommand); + if (result.getFirst() != 0) { + String msg = String.format( + "Failed to execute %s in network namespace for %s (PID = %d), exit code: %d, output: %s", + Arrays.toString(wrappedCommand), containerName.asString(), containerPid, result.getFirst(), result.getSecond()); logger.error(msg); throw new RuntimeException(msg); } - } catch (DockerExecTimeoutException e) { - logger.warning(String.format("Timed out while executing %s in network namespace for %s (PID = %d)", - Arrays.toString(wrappedCommand), containerName.asString(), containerPid)); - throw e; + } catch (IOException e) { + logger.warning(String.format("IOException while executing %s in network namespace for %s (PID = %d)", + Arrays.toString(wrappedCommand), containerName.asString(), containerPid), e); + throw new RuntimeException(e); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java index 7e8136403d8..99c78245b99 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java @@ -4,6 +4,8 @@ package com.yahoo.vespa.hosted.node.admin.provider; import com.google.inject.Inject; import com.yahoo.net.HostName; import static com.yahoo.vespa.defaults.Defaults.getDefaults; + +import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.Docker; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; @@ -60,7 +62,7 @@ public class ComponentsProviderImpl implements ComponentsProvider { ConfigServerHttpRequestExecutor requestExecutor = ConfigServerHttpRequestExecutor.create(configServerHosts); Orchestrator orchestrator = new OrchestratorImpl(requestExecutor); NodeRepository nodeRepository = new NodeRepositoryImpl(requestExecutor, WEB_SERVICE_PORT, baseHostName); - DockerOperations dockerOperations = new DockerOperationsImpl(docker, environment); + DockerOperations dockerOperations = new DockerOperationsImpl(docker, environment, new ProcessExecuter()); Optional<StorageMaintainer> storageMaintainer = environment.isRunningLocally() ? Optional.empty() : Optional.of(new StorageMaintainer(docker, metricReceiver, environment, clock)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java index 0ffc69f8698..93f045efd9c 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.docker; +import com.yahoo.collections.Pair; +import com.yahoo.system.ProcessExecuter; import com.yahoo.vespa.hosted.dockerapi.Container; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.Docker; @@ -11,10 +13,12 @@ import org.hamcrest.CoreMatchers; import org.junit.Test; import org.mockito.InOrder; +import java.io.IOException; import java.util.Optional; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertThat; +import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyVararg; import static org.mockito.Matchers.eq; @@ -26,7 +30,8 @@ import static org.mockito.Mockito.when; public class DockerOperationsImplTest { private final Environment environment = new Environment.Builder().build(); private final Docker docker = mock(Docker.class); - private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment); + private final ProcessExecuter processExecuter = mock(ProcessExecuter.class); + private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment, processExecuter); @Test public void processResultFromNodeProgramWhenSuccess() throws Exception { @@ -100,11 +105,13 @@ public class DockerOperationsImplTest { @Test public void runsCommandInNetworkNamespace() { Container container = makeContainer("container-42", Container.State.RUNNING, 42); - DockerOperationsImpl dockerOperations = new DockerOperationsImpl(docker, environment); - when(docker.executeInContainerAsRoot(eq(new ContainerName("node-admin")), eq(60L), - eq("nsenter"), eq("--net=/host/proc/42/ns/net"), eq("--"), eq("iptables"), eq("-nvL"))) - .thenReturn(new ProcessResult(0, "", "")); + try { + when(processExecuter.exec(aryEq(new String[]{"sudo", "nsenter", "--net=/host/proc/42/ns/net", "--", "iptables", "-nvL"}))) + .thenReturn(new Pair<>(0, "")); + } catch (IOException e) { + e.printStackTrace(); + } dockerOperations.executeCommandInNetworkNamespace(container.name, "iptables", "-nvL"); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java index 473d306e5bd..15be1209965 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java @@ -56,7 +56,7 @@ public class DockerTester implements AutoCloseable { MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); - final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment); + final DockerOperations dockerOperations = new DockerOperationsImpl(dockerMock, environment, null); Function<String, NodeAgent> nodeAgentFactory = (hostName) -> new NodeAgentImpl(hostName, nodeRepositoryMock, orchestratorMock, dockerOperations, Optional.of(storageMaintainer), environment, clock, Optional.empty()); nodeAdmin = new NodeAdminImpl(dockerOperations, nodeAgentFactory, Optional.of(storageMaintainer), 100, mr, Optional.empty(), Clock.systemUTC()); |