diff options
6 files changed, 65 insertions, 54 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 615eddac645..99a7c5abfa0 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 @@ -25,7 +25,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index de4aacff09f..6cfa5efcc81 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -6,12 +6,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.collections.Pair; import com.yahoo.io.IOUtils; -import com.yahoo.log.LogLevel; import com.yahoo.net.HostName; 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.ProcessResult; import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper; import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; @@ -30,6 +28,7 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -38,6 +37,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.regex.Pattern; +import java.util.stream.Stream; import static com.yahoo.vespa.defaults.Defaults.getDefaults; @@ -52,14 +52,16 @@ public class StorageMaintainer { private final Logger logger = Logger.getLogger(StorageMaintainer.class.getName()); private final CounterWrapper numberOfNodeAdminMaintenanceFails; private final Docker docker; + private final ProcessExecuter processExecuter; private final Environment environment; private final Clock clock; private Map<ContainerName, MaintenanceThrottler> maintenanceThrottlerByContainerName = new ConcurrentHashMap<>(); - public StorageMaintainer(Docker docker, MetricReceiverWrapper metricReceiver, Environment environment, Clock clock) { + public StorageMaintainer(Docker docker, ProcessExecuter processExecuter, MetricReceiverWrapper metricReceiver, Environment environment, Clock clock) { this.docker = docker; + this.processExecuter = processExecuter; this.environment = environment; this.clock = clock; @@ -157,14 +159,6 @@ public class StorageMaintainer { return diskUsageKB * 1024; } - Optional<String> readMeminfo() { - try { - return Optional.of(new String(Files.readAllBytes(Paths.get("/proc/meminfo")))); - } catch (IOException e) { - logger.log(LogLevel.WARNING, "Failed to read meminfo", e); - return Optional.empty(); - } - } /** * Deletes old log files for vespa, nginx, logstash, etc. @@ -317,20 +311,33 @@ public class StorageMaintainer { return kernelVersion.orElse("unknown"); } + + private String executeMaintainer(String mainClass, String... args) { + String[] command = Stream.concat( + Stream.of("sudo", "-E", getDefaults().underVespaHome("libexec/vespa/node-admin/maintenance.sh"), mainClass), + Stream.of(args)) + .toArray(String[]::new); + + try { + Pair<Integer, String> result = processExecuter.exec(command); + + if (result.getFirst() != 0) { + numberOfNodeAdminMaintenanceFails.add(); + throw new RuntimeException( + String.format("Maintainer failed to execute command: %s, Exit code: %d, Stdout/stderr: %s", + Arrays.toString(command), result.getFirst(), result.getSecond())); + } + return result.getSecond(); + } catch (IOException e) { + throw new RuntimeException("Failed to execute maintainer", e); + } + } + /** * Wrapper for node-admin-maintenance, queues up maintenances jobs and sends a single request to maintenance JVM */ private class MaintainerExecutor { private final List<MaintainerExecutorJob> jobs = new ArrayList<>(); - private final ContainerName executeIn; - - MaintainerExecutor(ContainerName executeIn) { - this.executeIn = executeIn; - } - - MaintainerExecutor() { - this(NODE_ADMIN); - } MaintainerExecutorJob addJob(String jobName) { MaintainerExecutorJob job = new MaintainerExecutorJob(jobName); @@ -346,16 +353,7 @@ public class StorageMaintainer { throw new RuntimeException("Failed transform list of maintenance jobs to JSON"); } - String[] command = {"java", - "-cp", getDefaults().underVespaHome("lib/jars/node-maintainer-jar-with-dependencies.jar"), - "-Dvespa.log.target=file:" + getDefaults().underVespaHome("logs/vespa/maintainer.log"), - "com.yahoo.vespa.hosted.node.maintainer.Maintainer", args}; - ProcessResult result = docker.executeInContainerAsRoot(executeIn, command); - - if (! result.isSuccess()) { - numberOfNodeAdminMaintenanceFails.add(); - throw new RuntimeException("Failed to run maintenance jobs: " + args + result); - } + executeMaintainer("com.yahoo.vespa.hosted.node.maintainer.Maintainer", args); } } 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 99c78245b99..56c15804179 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 @@ -59,13 +59,14 @@ public class ComponentsProviderImpl implements ComponentsProvider { } Clock clock = Clock.systemUTC(); + ProcessExecuter processExecuter = new ProcessExecuter(); 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, new ProcessExecuter()); + DockerOperations dockerOperations = new DockerOperationsImpl(docker, environment, processExecuter); Optional<StorageMaintainer> storageMaintainer = environment.isRunningLocally() ? - Optional.empty() : Optional.of(new StorageMaintainer(docker, metricReceiver, environment, clock)); + Optional.empty() : Optional.of(new StorageMaintainer(docker, processExecuter, metricReceiver, environment, clock)); Optional<AclMaintainer> aclMaintainer = environment.isRunningLocally() ? Optional.empty() : Optional.of(new AclMaintainer(dockerOperations, nodeRepository, baseHostName)); 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 15be1209965..735e8a574f7 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 @@ -52,7 +52,7 @@ public class DockerTester implements AutoCloseable { .inetAddressResolver(inetAddressResolver) .pathResolver(new PathResolver(Paths.get("/tmp"), Paths.get("/tmp"))).build(); Clock clock = Clock.systemUTC(); - StorageMaintainerMock storageMaintainer = new StorageMaintainerMock(dockerMock, environment, callOrderVerifier, clock); + StorageMaintainerMock storageMaintainer = new StorageMaintainerMock(dockerMock, null, environment, callOrderVerifier, clock); MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java index 04dc4010d8a..a7ce706063a 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.integrationTests; import com.yahoo.metrics.simple.MetricReceiver; +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; @@ -18,8 +19,8 @@ import java.util.Optional; public class StorageMaintainerMock extends StorageMaintainer { private final CallOrderVerifier callOrderVerifier; - public StorageMaintainerMock(Docker docker, Environment environment, CallOrderVerifier callOrderVerifier, Clock clock) { - super(docker, new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock); + public StorageMaintainerMock(Docker docker, ProcessExecuter processExecuter, Environment environment, CallOrderVerifier callOrderVerifier, Clock clock) { + super(docker, processExecuter, new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock); this.callOrderVerifier = callOrderVerifier; } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index c4b0dfbe153..0324ff285d1 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -1,11 +1,12 @@ // 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.maintenance; +import com.yahoo.collections.Pair; import com.yahoo.metrics.simple.MetricReceiver; +import com.yahoo.system.ProcessExecuter; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.dockerapi.Docker; -import com.yahoo.vespa.hosted.dockerapi.ProcessResult; import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; import com.yahoo.vespa.hosted.node.admin.util.Environment; @@ -22,7 +23,6 @@ import java.time.Duration; import static org.junit.Assert.*; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyVararg; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -36,7 +36,8 @@ public class StorageMaintainerTest { private final Environment environment = new Environment.Builder() .pathResolver(new PathResolver()).build(); private final Docker docker = mock(Docker.class); - private final StorageMaintainer storageMaintainer = new StorageMaintainer(docker, + private final ProcessExecuter processExecuter = mock(ProcessExecuter.class); + private final StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter, new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock); @Rule @@ -62,37 +63,39 @@ public class StorageMaintainerTest { .nodeType("tenants") .nodeFlavor("docker").build(); - when(docker.executeInContainerAsRoot(any(), anyVararg())).thenReturn(new ProcessResult(0, "", "")); + try { + when(processExecuter.exec(any(String[].class))).thenReturn(new Pair<>(0, "")); + } catch (IOException ignored) { } storageMaintainer.removeOldFilesFromNode(containerName); - verify(docker, times(1)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(1); // Will not actually run maintenance job until an hour passes storageMaintainer.removeOldFilesFromNode(containerName); - verify(docker, times(1)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(1); // Coredump handler has its own throttler storageMaintainer.handleCoreDumpsForContainer(containerName, nodeSpec, environment); - verify(docker, times(2)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(2); clock.advance(Duration.ofMinutes(61)); storageMaintainer.removeOldFilesFromNode(containerName); - verify(docker, times(3)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(3); storageMaintainer.handleCoreDumpsForContainer(containerName, nodeSpec, environment); - verify(docker, times(4)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(4); storageMaintainer.handleCoreDumpsForContainer(containerName, nodeSpec, environment); - verify(docker, times(4)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(4); // archiveNodeData is unthrottled and it should reset previous times storageMaintainer.archiveNodeData(containerName); - verify(docker, times(5)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(5); storageMaintainer.archiveNodeData(containerName); - verify(docker, times(6)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(6); storageMaintainer.handleCoreDumpsForContainer(containerName, nodeSpec, environment); - verify(docker, times(7)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(7); } @Test @@ -100,21 +103,30 @@ public class StorageMaintainerTest { String hostname = "node-123.us-north-3.test.yahoo.com"; ContainerName containerName = ContainerName.fromHostname(hostname); - when(docker.executeInContainerAsRoot(any(), anyVararg())) - .thenThrow(new RuntimeException("Something went wrong")) - .thenReturn(new ProcessResult(0, "", "")); + try { + when(processExecuter.exec(any(String[].class))) + .thenThrow(new RuntimeException("Something went wrong")) + .thenReturn(new Pair<>(0, "")); + } catch (IOException ignored) { } + try { storageMaintainer.removeOldFilesFromNode(containerName); fail("Maintenance job should've failed!"); } catch (RuntimeException ignored) { } - verify(docker, times(1)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(1); // Maintenance job failed, we should be able to immediately re-run it storageMaintainer.removeOldFilesFromNode(containerName); - verify(docker, times(2)).executeInContainerAsRoot(any(), anyVararg()); + verifyProcessExecuterCalled(2); } private static void writeNBytesToFile(File file, int nBytes) throws IOException { Files.write(file.toPath(), new byte[nBytes]); } + + private void verifyProcessExecuterCalled(int times) { + try { + verify(processExecuter, times(times)).exec(any(String[].class)); + } catch (IOException ignored) { } + } } |