diff options
42 files changed, 1783 insertions, 265 deletions
diff --git a/client/go/Makefile b/client/go/Makefile index 08f88960e4a..7f24d4ad479 100644 --- a/client/go/Makefile +++ b/client/go/Makefile @@ -29,7 +29,7 @@ all: test checkfmt install # $ git checkout vX.Y.Z # $ make dist-homebrew dist-homebrew: dist-version - brew bump-formula-pr --tag v$(VERSION) vespa-cli + brew bump-formula-pr --version $(VERSION) vespa-cli # Create a GitHub release draft for all platforms. Note that this only creates a # draft, which is not publicly visible until it's explicitly published. diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java index 1c26f532971..8c31eabfde0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java @@ -47,7 +47,7 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro @Override public void getConfig(ServerConfig.Builder builder) { builder.metric(new ServerConfig.Metric.Builder() - .monitoringHandlerPaths(List.of("/state/v1", "/status.html")) + .monitoringHandlerPaths(List.of("/state/v1", "/status.html", "/metrics/v2")) .searchHandlerPaths(List.of("/search")) ); if (isHostedVespa) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ActivationConflictException.java b/config-provisioning/src/main/java/com/yahoo/config/provision/exception/ActivationConflictException.java index 8dac35616f0..c548add8ddf 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ActivationConflictException.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/exception/ActivationConflictException.java @@ -1,5 +1,5 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server; +package com.yahoo.config.provision.exception; /** * Exception used when activation cannot be done because activation is for @@ -9,7 +9,9 @@ package com.yahoo.vespa.config.server; * @author hmusum */ public class ActivationConflictException extends RuntimeException { - public ActivationConflictException(String s) { - super(s); - } + + public ActivationConflictException(String message) { super(message); } + + public ActivationConflictException(String message, Throwable cause) { super(message, cause); } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 368324fa843..e0629a2e5db 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -22,6 +22,7 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provision.exception.ActivationConflictException; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.SecretStoreProvider; import com.yahoo.container.jdisc.secretstore.SecretStore; @@ -486,7 +487,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye // Config generation is equal to session id, and config generation must be a monotonically increasing number static void checkIfActiveIsNewerThanSessionToBeActivated(long sessionId, long currentActiveSessionId) { if (sessionId < currentActiveSessionId) { - throw new ActivationConflictException("It is not possible to activate session " + sessionId + + throw new ActivationConflictException("Cannot activate session " + sessionId + ", because it is older than current active session (" + currentActiveSessionId + ")"); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java index ea7d0e03812..2dad2c060cc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java @@ -3,19 +3,19 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.CertificateNotReadyException; +import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.ParentHostUnavailableException; +import com.yahoo.config.provision.exception.ActivationConflictException; import com.yahoo.config.provision.exception.LoadBalancerServiceException; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; -import java.util.logging.Level; -import com.yahoo.config.provision.OutOfCapacityException; -import com.yahoo.vespa.config.server.ActivationConflictException; import com.yahoo.yolean.Exceptions; import java.io.PrintWriter; import java.io.StringWriter; import java.time.Duration; +import java.util.logging.Level; /** * Super class for http handlers, that takes care of checking valid diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index 28d50a5396e..133570cc109 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -17,6 +17,7 @@ import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.NetworkPorts; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.exception.ActivationConflictException; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.Metric; diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java index 8d07e7c3757..638336e51d8 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java +++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java @@ -60,7 +60,7 @@ public class DefaultContainerThreadpool extends AbstractComponent implements Aut executor.prestartAllCoreThreads(); threadpool = new ExecutorServiceWrapper( executor, threadPoolMetric, processTerminator, config.maxThreadExecutionTimeSeconds() * 1000L, - name, queueSize); + name); } @Override public Executor executor() { return threadpool; } diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java index 52936f7bbef..5ba5985db37 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java @@ -32,38 +32,51 @@ class ExecutorServiceWrapper extends ForwardingExecutorService { ThreadPoolMetric metric, ProcessTerminator processTerminator, long maxThreadExecutionTimeMillis, - String name, - int queueCapacity) { + String name) { this.wrapped = wrapped; this.metric = metric; this.processTerminator = processTerminator; this.maxThreadExecutionTimeMillis = maxThreadExecutionTimeMillis; - this.queueCapacity = queueCapacity; + this.queueCapacity = wrapped.getMaximumPoolSize() + wrapped.getQueue().remainingCapacity() + wrapped.getQueue().size(); metric.reportThreadPoolSize(wrapped.getPoolSize()); metric.reportActiveThreads(wrapped.getActiveCount()); metricReporter = new Thread(this::reportMetrics); metricReporter.setName(name + "-threadpool-metric-reporter"); - metricReporter.setDaemon(true); metricReporter.start(); } private void reportMetrics() { - try { - while (!closed.get()) { - metric.reportThreadPoolSize(wrapped.getPoolSize()); - metric.reportActiveThreads(wrapped.getActiveCount()); - metric.reportWorkQueueSize(wrapped.getQueue().size()); - metric.reportWorkQueueCapacity(queueCapacity); - Thread.sleep(100); + while (timeToReportMetricsAgain(100)) { + metric.reportThreadPoolSize(wrapped.getPoolSize()); + metric.reportActiveThreads(wrapped.getActiveCount()); + metric.reportWorkQueueSize(wrapped.getQueue().size()); + metric.reportWorkQueueCapacity(queueCapacity); + } + } + private boolean timeToReportMetricsAgain(int timeoutMS) { + synchronized (closed) { + if (!closed.get()) { + try { + closed.wait(timeoutMS); + } catch (InterruptedException e) { + return false; + } } - } catch (InterruptedException e) { } + } + return !closed.get(); } @Override public void shutdown() { super.shutdown(); - closed.set(true); + synchronized (closed) { + closed.set(true); + closed.notify(); + } + try { + metricReporter.join(); + } catch (InterruptedException e) {} } /** diff --git a/dist/vespa.spec b/dist/vespa.spec index 7196fe92e20..e6eff61b214 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -78,7 +78,7 @@ BuildRequires: glibc-langpack-en BuildRequires: cmake3 BuildRequires: llvm7.0-devel BuildRequires: vespa-boost-devel >= 1.76.0-1 -BuildRequires: vespa-gtest >= 1.8.1-1 +BuildRequires: vespa-gtest = 1.11.0 %define _use_vespa_gtest 1 BuildRequires: vespa-icu-devel >= 65.1.0-1 BuildRequires: vespa-lz4-devel >= 1.9.2-2 @@ -108,7 +108,7 @@ BuildRequires: (llvm-devel >= 10.0.1 and llvm-devel < 11) BuildRequires: vespa-boost-devel >= 1.76.0-1 BuildRequires: vespa-openssl-devel >= 1.1.1l-1 %define _use_vespa_openssl 1 -BuildRequires: vespa-gtest >= 1.8.1-1 +BuildRequires: vespa-gtest = 1.11.0 %define _use_vespa_gtest 1 BuildRequires: vespa-lz4-devel >= 1.9.2-2 BuildRequires: vespa-onnxruntime-devel = 1.7.1 @@ -235,7 +235,7 @@ Requires: llvm7.0 Requires: vespa-telegraf >= 1.1.1-1 Requires: vespa-valgrind >= 3.17.0-1 %endif -Requires: vespa-gtest >= 1.8.1-1 +Requires: vespa-gtest = 1.11.0 %define _vespa_llvm_version 7 %define _extra_link_directory /usr/lib64/llvm7.0/lib;%{_vespa_deps_prefix}/lib64 %define _extra_include_directory /usr/include/llvm7.0;%{_vespa_deps_prefix}/include @@ -250,7 +250,7 @@ Requires: vespa-gtest >= 1.8.1-1 %else %define _vespa_llvm_version 10 %endif -Requires: vespa-gtest >= 1.8.1-1 +Requires: vespa-gtest = 1.11.0 %define _extra_link_directory %{_vespa_deps_prefix}/lib64 %define _extra_include_directory %{_vespa_deps_prefix}/include %endif 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 7d98a76dc6e..340f43b4671 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.node.admin.component.TaskContext; @@ -82,12 +83,13 @@ public class StorageMaintainer { public boolean syncLogs(NodeAgentContext context, boolean throttle) { Optional<URI> archiveUri = context.node().archiveUri(); if (archiveUri.isEmpty()) return false; + ApplicationId owner = context.node().owner().orElseThrow(); List<SyncFileInfo> syncFileInfos = FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa")) .maxDepth(2) .stream() .sorted(Comparator.comparing(FileFinder.FileAttributes::lastModifiedTime)) - .flatMap(fa -> SyncFileInfo.forLogFile(archiveUri.get(), fa.path(), throttle).stream()) + .flatMap(fa -> SyncFileInfo.forLogFile(archiveUri.get(), fa.path(), throttle, owner).stream()) .collect(Collectors.toList()); return syncClient.sync(context, syncFileInfos, throttle ? 1 : 100); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java new file mode 100644 index 00000000000..edab90afea7 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java @@ -0,0 +1,61 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; + +import java.nio.file.Path; +import java.util.Optional; + +/** + * An artifact file produced by a {@link ArtifactProducer}. + * + * @author bjorncs + */ +class Artifact { + + enum Classification { + CONFIDENTIAL("confidential"), + INTERNAL("internal"); + + private final String value; + Classification(String value) { this.value = value; } + public String value() { return value; } + } + + private final Classification classification; + private final Path fileInNode; + private final Path fileOnHost; + private final boolean compressOnUpload; + + private Artifact(Builder builder) { + if (builder.fileOnHost == null && builder.fileInNode == null) { + throw new IllegalArgumentException("No file specified"); + } else if (builder.fileOnHost != null && builder.fileInNode != null) { + throw new IllegalArgumentException("Only a single file can be specified"); + } + this.fileInNode = builder.fileInNode; + this.fileOnHost = builder.fileOnHost; + this.classification = builder.classification; + this.compressOnUpload = Boolean.TRUE.equals(builder.compressOnUpload); + } + + static Builder newBuilder() { return new Builder(); } + + Optional<Classification> classification() { return Optional.ofNullable(classification); } + Optional<Path> fileInNode() { return Optional.ofNullable(fileInNode); } + Optional<Path> fileOnHost() { return Optional.ofNullable(fileOnHost); } + boolean compressOnUpload() { return compressOnUpload; } + + static class Builder { + private Classification classification; + private Path fileInNode; + private Path fileOnHost; + private Boolean compressOnUpload; + + private Builder() {} + + Builder classification(Classification c) { this.classification = c; return this; } + Builder fileInNode(Path f) { this.fileInNode = f; return this; } + Builder fileOnHost(Path f) { this.fileOnHost = f; return this; } + Builder compressOnUpload() { this.compressOnUpload = true; return this; } + Artifact build() { return new Artifact(this); } + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java index 83e83f1f4d2..e4a9e6aeea5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java @@ -1,10 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; -import java.io.IOException; +import java.nio.file.Path; +import java.util.List; +import java.util.OptionalDouble; /** * Produces service dump artifacts. @@ -13,10 +14,25 @@ import java.io.IOException; */ interface ArtifactProducer { - String name(); + String artifactName(); + String description(); + List<Artifact> produceArtifacts(Context ctx); - void produceArtifact(NodeAgentContext context, String configId, ServiceDumpReport.DumpOptions options, - UnixPath resultDirectoryInNode) throws IOException; + interface Context { + String serviceId(); + int servicePid(); + CommandResult executeCommandInNode(List<String> command, boolean logOutput); + Path outputDirectoryInNode(); + Path pathInNodeUnderVespaHome(String relativePath); + Path pathOnHostFromPathInNode(Path pathInNode); + Options options(); + + interface Options { + OptionalDouble duration(); + boolean callGraphRecording(); + boolean sendProfilingSignal(); + } + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducers.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducers.java new file mode 100644 index 00000000000..4218df662da --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducers.java @@ -0,0 +1,107 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; + +import com.yahoo.yolean.concurrent.Sleeper; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * @author bjorncs + */ +class ArtifactProducers { + + private final Map<String, ArtifactProducer> producers; + private final Map<String, List<ArtifactProducer>> aliases; + + private ArtifactProducers(Set<ArtifactProducer> producers, + Map<String, List<Class<? extends ArtifactProducer>>> aliases) { + var producerMap = producers.stream() + .collect(Collectors.toMap(ArtifactProducer::artifactName, Function.identity())); + Map<String, List<ArtifactProducer>> aliasMap = new HashMap<>(); + aliases.forEach((alias, mapping) -> { + List<ArtifactProducer> concreteMapping = mapping.stream() + .map(type -> producers.stream() + .filter(p -> p.getClass().equals(type)) + .findAny() + .orElseThrow(() -> new IllegalArgumentException("No producer of type " + type))) + .collect(Collectors.toList()); + if (producerMap.containsKey(alias)) { + throw new IllegalStateException("Alias name '" + alias + "' conflicts with producer"); + } + aliasMap.put(alias, concreteMapping); + }); + this.producers = producerMap; + this.aliases = aliasMap; + } + + static ArtifactProducers createDefault(Sleeper sleeper) { + var producers = Set.of( + new PerfReporter(), + new JvmDumper.JavaFlightRecorder(sleeper), + new JvmDumper.HeapDump(), + new JvmDumper.Jmap(), + new JvmDumper.Jstat(), + new JvmDumper.Jstack(), + new PmapReporter(), + new VespaLogDumper(sleeper)); + var aliases = + Map.of( + "jvm-dump", + List.of( + JvmDumper.HeapDump.class, JvmDumper.Jmap.class, JvmDumper.Jstat.class, + JvmDumper.Jstack.class, VespaLogDumper.class) + ); + return new ArtifactProducers(producers, aliases); + } + + static ArtifactProducers createCustom(Set<ArtifactProducer> producers, + Map<String, List<Class<? extends ArtifactProducer>>> aliases) { + return new ArtifactProducers(producers, aliases); + } + + List<ArtifactProducer> resolve(List<String> requestedArtifacts) { + List<ArtifactProducer> resolved = new ArrayList<>(); + for (String artifact : requestedArtifacts) { + if (aliases.containsKey(artifact)) { + aliases.get(artifact).stream() + .filter(p -> !resolved.contains(p)) + .forEach(resolved::add); + } else if (producers.containsKey(artifact)) { + ArtifactProducer producer = producers.get(artifact); + if (!resolved.contains(producer)) { + resolved.add(producer); + } + } else { + throw createInvalidArtifactException(artifact); + } + } + return resolved; + } + + private IllegalArgumentException createInvalidArtifactException(String artifact) { + String producersString = producers.keySet().stream() + .map(a -> "'" + a + "'") + .sorted() + .collect(Collectors.joining(", ", "[", "]")); + String aliasesString = aliases.entrySet().stream() + .map(e -> String.format( + "'%s': %s", + e.getKey(), + e.getValue().stream() + .map(p -> "'" + p.artifactName() + "'") + .sorted() + .collect(Collectors.joining(", ", "[", "]"))) + ) + .collect(Collectors.joining(", ", "[", "]")); + String msg = String.format( + "Invalid artifact type '%s'. Valid types are %s and valid aliases are %s", + artifact, producersString, aliasesString); + return new IllegalArgumentException(msg); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java deleted file mode 100644 index 4d2b9d10069..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; - -import com.yahoo.yolean.concurrent.Sleeper; -import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; - -import java.io.IOException; -import java.time.Duration; -import java.util.List; - -/** - * Creates a Java Flight Recorder dump. - * - * @author bjorncs - */ -class JavaFlightRecorder extends AbstractProducer { - - private final Sleeper sleeper; - - JavaFlightRecorder(ContainerOperations container, Sleeper sleeper) { - super(container); - this.sleeper = sleeper; - } - - static String NAME = "jfr-recording"; - - @Override public String name() { return NAME; } - - @Override - public void produceArtifact(NodeAgentContext ctx, String configId, ServiceDumpReport.DumpOptions options, - UnixPath resultDirectoryInNode) throws IOException { - int pid = findVespaServicePid(ctx, configId); - int seconds = (int) (duration(ctx, options, 30.0)); - UnixPath outputFile = resultDirectoryInNode.resolve("recording.jfr"); - List<String> startCommand = List.of("jcmd", Integer.toString(pid), "JFR.start", "name=host-admin", - "path-to-gc-roots=true", "settings=profile", "filename=" + outputFile, "duration=" + seconds + "s"); - executeCommand(ctx, startCommand, true); - sleeper.sleep(Duration.ofSeconds(seconds).plusSeconds(1)); - int maxRetries = 10; - List<String> checkCommand = List.of("jcmd", Integer.toString(pid), "JFR.check", "name=host-admin"); - for (int i = 0; i < maxRetries; i++) { - boolean stillRunning = executeCommand(ctx, checkCommand, true).getOutputLines().stream() - .anyMatch(l -> l.contains("name=host-admin") && l.contains("running")); - if (!stillRunning) return; - sleeper.sleep(Duration.ofSeconds(1)); - } - throw new IOException("Failed to wait for JFR dump to complete after " + maxRetries + " retries"); - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java deleted file mode 100644 index 5cbbf304bb8..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; - -import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; - -import java.io.IOException; -import java.util.List; - -/** - * Creates a dump of JVM based Vespa services using vespa-jvm-dumper - * - * @author bjorncs - */ -class JvmDumpProducer extends AbstractProducer { - - JvmDumpProducer(ContainerOperations container) { super(container); } - - public static String NAME = "jvm-dump"; - - @Override public String name() { return NAME; } - - @Override - public void produceArtifact(NodeAgentContext context, String configId, ServiceDumpReport.DumpOptions options, - UnixPath resultDirectoryInNode) throws IOException { - UnixPath vespaJvmDumper = new UnixPath(context.pathInNodeUnderVespaHome("bin/vespa-jvm-dumper")); - executeCommand(context, List.of(vespaJvmDumper.toString(), configId, resultDirectoryInNode.toString()), true); - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java new file mode 100644 index 00000000000..cf206918568 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java @@ -0,0 +1,103 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; + +import com.yahoo.yolean.concurrent.Sleeper; + +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; + +import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.CONFIDENTIAL; +import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.INTERNAL; + +/** + * @author bjorncs + */ +class JvmDumper { + private JvmDumper() {} + + static class HeapDump implements ArtifactProducer { + @Override public String artifactName() { return "jvm-heap-dump"; } + @Override public String description() { return "JVM heap dump"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + Path heapDumpFile = ctx.outputDirectoryInNode().resolve("jvm-heap-dump.bin"); + List<String> cmd = List.of( + "jmap", "-dump:live,format=b,file=" + heapDumpFile, Integer.toString(ctx.servicePid())); + ctx.executeCommandInNode(cmd, true); + return List.of( + Artifact.newBuilder().classification(CONFIDENTIAL).fileInNode(heapDumpFile).compressOnUpload().build()); + } + } + + static class Jmap implements ArtifactProducer { + @Override public String artifactName() { return "jvm-jmap"; } + @Override public String description() { return "JVM jmap output"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + Path jmapReport = ctx.outputDirectoryInNode().resolve("jvm-jmap.txt"); + List<String> cmd = List.of("bash", "-c", "jhsdb jmap --heap --pid " + ctx.servicePid() + " > " + jmapReport); + ctx.executeCommandInNode(cmd, true); + return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(jmapReport).build()); + } + } + + static class Jstat implements ArtifactProducer { + @Override public String artifactName() { return "jvm-jstat"; } + @Override public String description() { return "JVM jstat output"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + Path jstatReport = ctx.outputDirectoryInNode().resolve("jvm-jstat.txt"); + List<String> cmd = List.of("bash", "-c", "jstat -gcutil " + ctx.servicePid() + " > " + jstatReport); + ctx.executeCommandInNode(cmd, true); + return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(jstatReport).build()); + } + } + + static class Jstack implements ArtifactProducer { + @Override public String artifactName() { return "jvm-jstack"; } + @Override public String description() { return "JVM jstack output"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + Path jstackReport = ctx.outputDirectoryInNode().resolve("jvm-jstack.txt"); + ctx.executeCommandInNode(List.of("bash", "-c", "jstack " + ctx.servicePid() + " > " + jstackReport), true); + return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(jstackReport).build()); + } + } + + static class JavaFlightRecorder implements ArtifactProducer { + private final Sleeper sleeper; + + JavaFlightRecorder(Sleeper sleeper) { this.sleeper = sleeper; } + + @Override public String artifactName() { return "jvm-jfr"; } + @Override public String description() { return "Java Flight Recorder recording"; } + + @Override + public List<Artifact> produceArtifacts(ArtifactProducer.Context ctx) { + int seconds = (int) (ctx.options().duration().orElse(30.0)); + Path outputFile = ctx.outputDirectoryInNode().resolve("recording.jfr"); + List<String> startCommand = List.of("jcmd", Integer.toString(ctx.servicePid()), "JFR.start", "name=host-admin", + "path-to-gc-roots=true", "settings=profile", "filename=" + outputFile, "duration=" + seconds + "s"); + ctx.executeCommandInNode(startCommand, true); + sleeper.sleep(Duration.ofSeconds(seconds).plusSeconds(1)); + int maxRetries = 10; + List<String> checkCommand = List.of("jcmd", Integer.toString(ctx.servicePid()), "JFR.check", "name=host-admin"); + for (int i = 0; i < maxRetries; i++) { + boolean stillRunning = ctx.executeCommandInNode(checkCommand, true).getOutputLines().stream() + .anyMatch(l -> l.contains("name=host-admin") && l.contains("running")); + if (!stillRunning) { + Artifact a = Artifact.newBuilder() + .classification(CONFIDENTIAL).fileInNode(outputFile).compressOnUpload().build(); + return List.of(a); + } + sleeper.sleep(Duration.ofSeconds(1)); + } + throw new RuntimeException("Failed to wait for JFR dump to complete after " + maxRetries + " retries"); + } + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReportProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReportProducer.java deleted file mode 100644 index 1370f09acac..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReportProducer.java +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; - -import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; -import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -/** - * @author bjorncs - */ -class PerfReportProducer extends AbstractProducer { - - public static String NAME = "perf-report"; - - PerfReportProducer(ContainerOperations container) { super(container); } - - @Override public String name() { return NAME; } - - @Override - public void produceArtifact(NodeAgentContext context, String configId, ServiceDumpReport.DumpOptions options, - UnixPath resultDirectoryInNode) throws IOException { - int pid = findVespaServicePid(context, configId); - int duration = (int) duration(context, options, 30.0); - List<String> perfRecordCommand = new ArrayList<>(List.of("perf", "record")); - if (options != null && Boolean.TRUE.equals(options.callGraphRecording())) { - perfRecordCommand.add("-g"); - } - String recordFile = resultDirectoryInNode.resolve("perf-record.bin").toString(); - perfRecordCommand.addAll( - List.of("--output=" + recordFile, - "--pid=" + pid, "sleep", Integer.toString(duration))); - executeCommand(context, perfRecordCommand, true); - String perfReportFile = resultDirectoryInNode.resolve("perf-report.txt").toString(); - executeCommand(context, List.of("bash", "-c", "perf report --input=" + recordFile + " > " + perfReportFile), true); - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java new file mode 100644 index 00000000000..07c8b709e04 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java @@ -0,0 +1,39 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.CONFIDENTIAL; +import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.INTERNAL; + +/** + * @author bjorncs + */ +class PerfReporter implements ArtifactProducer { + + PerfReporter() {} + + @Override public String artifactName() { return "perf-report"; } + @Override public String description() { return "Perf recording and report"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + int duration = (int)ctx.options().duration().orElse(30.0); + List<String> perfRecordCommand = new ArrayList<>(List.of("perf", "record")); + if (ctx.options().callGraphRecording()) { + perfRecordCommand.add("-g"); + } + Path recordFile = ctx.outputDirectoryInNode().resolve("perf-record.bin"); + perfRecordCommand.addAll( + List.of("--output=" + recordFile, + "--pid=" + ctx.servicePid(), "sleep", Integer.toString(duration))); + ctx.executeCommandInNode(perfRecordCommand, true); + Path reportFile = ctx.outputDirectoryInNode().resolve("perf-report.txt"); + ctx.executeCommandInNode(List.of("bash", "-c", "perf report --input=" + recordFile + " > " + reportFile), true); + return List.of( + Artifact.newBuilder().classification(CONFIDENTIAL).fileInNode(recordFile).compressOnUpload().build(), + Artifact.newBuilder().classification(INTERNAL).fileInNode(reportFile).build()); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java new file mode 100644 index 00000000000..659628d03a0 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; + +import java.nio.file.Path; +import java.util.List; + +import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.INTERNAL; + +/** + * @author bjorncs + */ +class PmapReporter implements ArtifactProducer { + @Override public String artifactName() { return "pmap"; } + @Override public String description() { return "Pmap report"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + Path pmapReport = ctx.outputDirectoryInNode().resolve("pmap.txt"); + List<String> cmd = List.of("bash", "-c", "pmap -x " + ctx.servicePid() + " > " + pmapReport); + ctx.executeCommandInNode(cmd, true); + return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(pmapReport).build()); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java index 09fac496b19..452f786301b 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ServiceDumpReport.java @@ -111,19 +111,24 @@ class ServiceDumpReport extends BaseReport { private static final String CALL_GRAPH_RECORDING_FIELD = "callGraphRecording"; private static final String DURATION_FIELD = "duration"; + private static final String SEND_PROFILING_SIGNAL_FIELD = "sendProfilingSignal"; private final Boolean callGraphRecording; private final Double duration; + private final Boolean sendProfilingSignal; @JsonCreator public DumpOptions(@JsonProperty(CALL_GRAPH_RECORDING_FIELD) Boolean callGraphRecording, - @JsonProperty(DURATION_FIELD) Double duration) { + @JsonProperty(DURATION_FIELD) Double duration, + @JsonProperty(SEND_PROFILING_SIGNAL_FIELD) Boolean sendProfilingSignal) { this.callGraphRecording = callGraphRecording; this.duration = duration; + this.sendProfilingSignal = sendProfilingSignal; } @JsonGetter(CALL_GRAPH_RECORDING_FIELD) public Boolean callGraphRecording() { return callGraphRecording; } @JsonGetter(DURATION_FIELD) public Double duration() { return duration; } + @JsonGetter(SEND_PROFILING_SIGNAL_FIELD) public Boolean sendProfilingSignal() { return sendProfilingSignal; } } @JsonIgnore public boolean isCompletedOrFailed() { return !isNullTimestamp(failedAt) || !isNullTimestamp(completedAt); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java new file mode 100644 index 00000000000..24224789877 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java @@ -0,0 +1,47 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; + +import com.yahoo.yolean.concurrent.Sleeper; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; +import java.util.logging.Logger; + +import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.CONFIDENTIAL; +import static com.yahoo.yolean.Exceptions.uncheck; + +/** + * @author bjorncs + */ +class VespaLogDumper implements ArtifactProducer { + + private static final Logger log = Logger.getLogger(VespaLogDumper.class.getName()); + + private final Sleeper sleeper; + + VespaLogDumper(Sleeper sleeper) { this.sleeper = sleeper; } + + @Override public String artifactName() { return "vespa-log"; } + @Override public String description() { return "Current Vespa logs"; } + + @Override + public List<Artifact> produceArtifacts(Context ctx) { + if (ctx.options().sendProfilingSignal()) { + log.info("Sending SIGPROF to process to include vespa-malloc dump in Vespa log"); + ctx.executeCommandInNode(List.of("kill", "-SIGPROF", Integer.toString(ctx.servicePid())), true); + sleeper.sleep(Duration.ofSeconds(3)); + } + Path vespaLogFile = ctx.pathOnHostFromPathInNode(ctx.pathInNodeUnderVespaHome("logs/vespa/vespa.log")); + Path destination = ctx.pathOnHostFromPathInNode(ctx.outputDirectoryInNode()).resolve("vespa.log"); + if (Files.exists(vespaLogFile)) { + uncheck(() -> Files.copy(vespaLogFile, destination)); + return List.of( + Artifact.newBuilder().classification(CONFIDENTIAL).fileOnHost(destination).compressOnUpload().build()); + } else { + log.info("Log file '" + vespaLogFile + "' does not exist"); + return List.of(); + } + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java index a202fb7acd0..23a6ed2aa8c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; -import com.yahoo.yolean.concurrent.Sleeper; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.text.Lowercase; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; @@ -9,18 +9,21 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncClient; import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo; +import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo.Compression; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; -import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; +import com.yahoo.yolean.concurrent.Sleeper; import java.net.URI; +import java.nio.file.Path; import java.time.Clock; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.List; -import java.util.Map; -import java.util.function.Function; +import java.util.Optional; +import java.util.OptionalDouble; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -40,25 +43,20 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { private final SyncClient syncClient; private final NodeRepository nodeRepository; private final Clock clock; - private final Map<String, ArtifactProducer> artifactProducers; + private final ArtifactProducers artifactProducers; public VespaServiceDumperImpl(ContainerOperations container, SyncClient syncClient, NodeRepository nodeRepository) { - this(container, syncClient, nodeRepository, Clock.systemUTC(), Sleeper.DEFAULT); + this(ArtifactProducers.createDefault(Sleeper.DEFAULT), container, syncClient, nodeRepository, Clock.systemUTC()); } // For unit testing - VespaServiceDumperImpl(ContainerOperations container, SyncClient syncClient, NodeRepository nodeRepository, - Clock clock, Sleeper sleeper) { + VespaServiceDumperImpl(ArtifactProducers producers, ContainerOperations container, SyncClient syncClient, + NodeRepository nodeRepository, Clock clock) { this.container = container; this.syncClient = syncClient; this.nodeRepository = nodeRepository; this.clock = clock; - List<AbstractProducer> producers = List.of( - new JvmDumpProducer(container), - new PerfReportProducer(container), - new JavaFlightRecorder(container, sleeper)); - this.artifactProducers = producers.stream() - .collect(Collectors.toMap(ArtifactProducer::name, Function.identity())); + this.artifactProducers = producers; } @Override @@ -85,8 +83,8 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { handleFailure(context, request, startedAt, "Request already expired"); return; } - List<String> artifactTypes = request.artifacts(); - if (artifactTypes == null || artifactTypes.isEmpty()) { + List<String> requestedArtifacts = request.artifacts(); + if (requestedArtifacts == null || requestedArtifacts.isEmpty()) { handleFailure(context, request, startedAt, "No artifacts requested"); return; } @@ -104,30 +102,14 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { context.log(log, Level.INFO, "Creating '" + directoryOnHost +"'."); directoryOnHost.createDirectory(); directoryOnHost.setPermissions("rwxrwxrwx"); - List<SyncFileInfo> files = new ArrayList<>(); URI destination = serviceDumpDestination(nodeSpec, createDumpId(request)); - for (String artifactType : artifactTypes) { - ArtifactProducer producer = artifactProducers.get(artifactType); - if (producer == null) { - String supportedValues = String.join(",", artifactProducers.keySet()); - handleFailure(context, request, startedAt, "No artifact producer exists for '" + artifactType + "'. " + - "Following values are allowed: " + supportedValues); - return; - } - context.log(log, "Producing artifact of type '" + artifactType + "'"); - UnixPath producerDirectoryOnHost = directoryOnHost.resolve(artifactType); - producerDirectoryOnHost.createDirectory(); - producerDirectoryOnHost.setPermissions("rwxrwxrwx"); - UnixPath producerDirectoryInNode = directoryInNode.resolve(artifactType); - producer.produceArtifact(context, configId, request.dumpOptions(), producerDirectoryInNode); - collectArtifactFilesToUpload(files, producerDirectoryOnHost, destination.resolve(artifactType + '/'), expiry); + ProducerContext producerCtx = new ProducerContext(context, directoryInNode.toPath(), request); + List<Artifact> producedArtifacts = new ArrayList<>(); + for (ArtifactProducer producer : artifactProducers.resolve(requestedArtifacts)) { + context.log(log, "Producing artifact of type '" + producer.artifactName() + "'"); + producedArtifacts.addAll(producer.produceArtifacts(producerCtx)); } - context.log(log, Level.INFO, "Uploading files with destination " + destination + " and expiry " + expiry); - if (!syncClient.sync(context, files, Integer.MAX_VALUE)) { - handleFailure(context, request, startedAt, "Unable to upload all files"); - return; - } - context.log(log, Level.INFO, "Upload complete"); + uploadArtifacts(context, destination, producedArtifacts, expiry); storeReport(context, ServiceDumpReport.createSuccessReport(request, startedAt, clock.instant(), destination)); } catch (Exception e) { handleFailure(context, request, startedAt, e); @@ -139,10 +121,24 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { } } - private void collectArtifactFilesToUpload(List<SyncFileInfo> files, UnixPath directoryOnHost, URI destination, Instant expiry) { - FileFinder.files(directoryOnHost.toPath()).stream() - .flatMap(file -> SyncFileInfo.forServiceDump(destination, file.path(), expiry).stream()) - .forEach(files::add); + private void uploadArtifacts(NodeAgentContext ctx, URI destination, + List<Artifact> producedArtifacts, Instant expiry) { + ApplicationId owner = ctx.node().owner().orElseThrow(); + List<SyncFileInfo> filesToUpload = producedArtifacts.stream() + .map(a -> { + Compression compression = a.compressOnUpload() ? Compression.ZSTD : Compression.NONE; + Path fileInNode = a.fileInNode().orElse(null); + Path fileOnHost = fileInNode != null ? ctx.pathOnHostFromPathInNode(fileInNode) : a.fileOnHost().orElseThrow(); + String classification = a.classification().map(Artifact.Classification::value).orElse(null); + return SyncFileInfo.forServiceDump(destination, fileOnHost, expiry, compression, owner, classification); + }) + .collect(Collectors.toList()); + ctx.log(log, Level.INFO, + "Uploading " + filesToUpload.size() + " file(s) with destination " + destination + " and expiry " + expiry); + if (!syncClient.sync(ctx, filesToUpload, Integer.MAX_VALUE)) { + throw new RuntimeException("Unable to upload all files"); + } + ctx.log(log, Level.INFO, "Upload complete"); } private static Instant expireAt(Instant startedAt, ServiceDumpReport request) { @@ -181,5 +177,86 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { return archiveUri.resolve(targetDirectory); } + private class ProducerContext implements ArtifactProducer.Context, ArtifactProducer.Context.Options { + + final NodeAgentContext nodeAgentCtx; + final Path outputDirectoryInNode; + final ServiceDumpReport request; + volatile int pid = -1; + + ProducerContext(NodeAgentContext nodeAgentCtx, Path outputDirectoryInNode, ServiceDumpReport request) { + this.nodeAgentCtx = nodeAgentCtx; + this.outputDirectoryInNode = outputDirectoryInNode; + this.request = request; + } + + @Override public String serviceId() { return request.configId(); } + + @Override + public int servicePid() { + if (pid == -1) { + Path findPidBinary = nodeAgentCtx.pathInNodeUnderVespaHome("libexec/vespa/find-pid"); + CommandResult findPidResult = executeCommandInNode(List.of(findPidBinary.toString(), serviceId()), true); + this.pid = Integer.parseInt(findPidResult.getOutput()); + } + return pid; + } + + @Override + public CommandResult executeCommandInNode(List<String> command, boolean logOutput) { + CommandResult result = container.executeCommandInContainerAsRoot(nodeAgentCtx, command.toArray(new String[0])); + String cmdString = command.stream().map(s -> "'" + s + "'").collect(Collectors.joining(" ", "\"", "\"")); + int exitCode = result.getExitCode(); + String output = result.getOutput().trim(); + String prefixedOutput = output.contains("\n") + ? "\n" + output + : (output.isEmpty() ? "<no output>" : output); + if (exitCode > 0) { + String errorMsg = logOutput + ? String.format("Failed to execute %s (exited with code %d): %s", cmdString, exitCode, prefixedOutput) + : String.format("Failed to execute %s (exited with code %d)", cmdString, exitCode); + throw new RuntimeException(errorMsg); + } else { + String logMsg = logOutput + ? String.format("Executed command %s. Exited with code %d and output: %s", cmdString, exitCode, prefixedOutput) + : String.format("Executed command %s. Exited with code %d.", cmdString, exitCode); + nodeAgentCtx.log(log, logMsg); + } + return result; + } + @Override public Path outputDirectoryInNode() { return outputDirectoryInNode; } + + @Override + public Path pathInNodeUnderVespaHome(String relativePath) { + return nodeAgentCtx.pathInNodeUnderVespaHome(relativePath); + } + + @Override + public Path pathOnHostFromPathInNode(Path pathInNode) { + return nodeAgentCtx.pathOnHostFromPathInNode(pathInNode); + } + + @Override public Options options() { return this; } + + @Override + public OptionalDouble duration() { + Double duration = dumpOptions() + .map(ServiceDumpReport.DumpOptions::duration) + .orElse(null); + return duration != null ? OptionalDouble.of(duration) : OptionalDouble.empty(); + } + + @Override + public boolean callGraphRecording() { + return dumpOptions().map(ServiceDumpReport.DumpOptions::callGraphRecording).orElse(false); + } + + @Override + public boolean sendProfilingSignal() { + return dumpOptions().map(ServiceDumpReport.DumpOptions::sendProfilingSignal).orElse(false); + } + + Optional<ServiceDumpReport.DumpOptions> dumpOptions() { return Optional.ofNullable(request.dumpOptions()); } + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java index 8b05425d2d9..6d87a8cfc4a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java @@ -1,10 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance.sync; +import com.yahoo.config.provision.ApplicationId; + import java.net.URI; import java.nio.file.Path; import java.time.Instant; -import java.util.List; +import java.time.temporal.ChronoUnit; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; /** @@ -16,12 +20,15 @@ public class SyncFileInfo { private final URI destination; private final Compression uploadCompression; private final Instant expiry; + private final Map<String, String> tags; - private SyncFileInfo(Path source, URI destination, Compression uploadCompression, Instant expiry) { + private SyncFileInfo(Path source, URI destination, Compression uploadCompression, Instant expiry, + Map<String, String> tags) { this.source = source; this.destination = destination; this.uploadCompression = uploadCompression; this.expiry = expiry; + this.tags = Map.copyOf(tags); } /** Source path of the file to sync */ @@ -42,7 +49,9 @@ public class SyncFileInfo { /** File expiry */ public Optional<Instant> expiry() { return Optional.ofNullable(expiry); } - public static Optional<SyncFileInfo> forLogFile(URI uri, Path logFile, boolean rotatedOnly) { + public Map<String, String> tags() { return tags; } + + public static Optional<SyncFileInfo> forLogFile(URI uri, Path logFile, boolean rotatedOnly, ApplicationId owner) { String filename = logFile.getFileName().toString(); Compression compression; String dir = null; @@ -61,17 +70,26 @@ public class SyncFileInfo { } if (dir == null) return Optional.empty(); + Instant expiry = Instant.now().plus(30, ChronoUnit.DAYS); return Optional.of(new SyncFileInfo( - logFile, uri.resolve(dir + logFile.getFileName() + compression.extension), compression, null)); + logFile, uri.resolve(dir + logFile.getFileName() + compression.extension), compression, expiry, defaultTags(owner))); } - public static Optional<SyncFileInfo> forServiceDump(URI directory, Path file, Instant expiry) { + public static SyncFileInfo forServiceDump(URI destinationDir, Path file, Instant expiry, Compression compression, + ApplicationId owner, String assetClassification) { String filename = file.getFileName().toString(); - List<String> filesToCompress = List.of(".bin", ".hprof", ".jfr", ".log"); - Compression compression = filesToCompress.stream().anyMatch(filename::endsWith) ? Compression.ZSTD : Compression.NONE; - if (filename.startsWith(".")) return Optional.empty(); - URI location = directory.resolve(filename + compression.extension); - return Optional.of(new SyncFileInfo(file, location, compression, expiry)); + URI location = destinationDir.resolve(filename + compression.extension); + Map<String, String> tags = defaultTags(owner); + if (assetClassification != null) { + tags.put("vespa:AssetClassification", assetClassification); + } + return new SyncFileInfo(file, location, compression, expiry, tags); + } + + private static Map<String, String> defaultTags(ApplicationId owner) { + var tags = new HashMap<String, String>(); + tags.put("corp:Application", owner.toFullString()); + return tags; } public enum Compression { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java index 1121db99399..2ce49ae383a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java @@ -149,7 +149,7 @@ public class UnixPath { public UnixPath setGroup(String group) { return setGroup(group, "group"); } public UnixPath setGroupId(int gid) { return setGroup(String.valueOf(gid), "gid"); } - public UnixPath setGroup(String group, String type) { + private UnixPath setGroup(String group, String type) { UserPrincipalLookupService service = path.getFileSystem().getUserPrincipalLookupService(); GroupPrincipal principal = uncheck( () -> service.lookupPrincipalByGroupName(group), diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerAttributeViews.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerAttributeViews.java new file mode 100644 index 00000000000..c3246843c75 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerAttributeViews.java @@ -0,0 +1,81 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import java.io.IOException; +import java.nio.file.ProviderMismatchException; +import java.nio.file.attribute.FileTime; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.UserPrincipal; +import java.util.Map; +import java.util.Set; + +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.ContainerGroupPrincipal; +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.ContainerUserPrincipal; + +/** + * @author valerijf + */ +class ContainerAttributeViews { + + static class ContainerPosixFileAttributeView implements PosixFileAttributeView { + private final PosixFileAttributeView posixFileAttributeView; + private final ContainerPosixFileAttributes fileAttributes; + + ContainerPosixFileAttributeView(PosixFileAttributeView posixFileAttributeView, + ContainerPosixFileAttributes fileAttributes) { + this.posixFileAttributeView = posixFileAttributeView; + this.fileAttributes = fileAttributes; + } + + @Override public String name() { return "posix"; } + @Override public UserPrincipal getOwner() { return fileAttributes.owner(); } + @Override public PosixFileAttributes readAttributes() { return fileAttributes; } + + @Override + public void setOwner(UserPrincipal owner) throws IOException { + if (!(owner instanceof ContainerUserPrincipal)) throw new ProviderMismatchException(); + posixFileAttributeView.setOwner(((ContainerUserPrincipal) owner).baseFsPrincipal()); + } + + @Override + public void setGroup(GroupPrincipal group) throws IOException { + if (!(group instanceof ContainerGroupPrincipal)) throw new ProviderMismatchException(); + posixFileAttributeView.setGroup(((ContainerGroupPrincipal) group).baseFsPrincipal()); + } + + @Override + public void setTimes(FileTime lastModifiedTime, FileTime lastAccessTime, FileTime createTime) throws IOException { + posixFileAttributeView.setTimes(lastModifiedTime, lastAccessTime, createTime); + } + + @Override + public void setPermissions(Set<PosixFilePermission> perms) throws IOException { + posixFileAttributeView.setPermissions(perms); + } + } + + static class ContainerPosixFileAttributes implements PosixFileAttributes { + private final Map<String, Object> attributes; + + ContainerPosixFileAttributes(Map<String, Object> attributes) { + this.attributes = attributes; + } + + @SuppressWarnings("unchecked") + @Override public Set<PosixFilePermission> permissions() { return (Set<PosixFilePermission>) attributes.get("permissions"); } + @Override public ContainerUserPrincipal owner() { return (ContainerUserPrincipal) attributes.get("owner"); } + @Override public ContainerGroupPrincipal group() { return (ContainerGroupPrincipal) attributes.get("group"); } + @Override public FileTime lastModifiedTime() { return (FileTime) attributes.get("lastModifiedTime"); } + @Override public FileTime lastAccessTime() { return (FileTime) attributes.get("lastAccessTime"); } + @Override public FileTime creationTime() { return (FileTime) attributes.get("creationTime"); } + @Override public boolean isRegularFile() { return (boolean) attributes.get("isRegularFile"); } + @Override public boolean isDirectory() { return (boolean) attributes.get("isDirectory"); } + @Override public boolean isSymbolicLink() { return (boolean) attributes.get("isSymbolicLink"); } + @Override public boolean isOther() { return (boolean) attributes.get("isOther"); } + @Override public long size() { return (long) attributes.get("size"); } + @Override public Object fileKey() { return attributes.get("fileKey"); } + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java new file mode 100644 index 00000000000..393137f795e --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java @@ -0,0 +1,84 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import java.io.IOException; +import java.nio.file.FileStore; +import java.nio.file.FileSystem; +import java.nio.file.Path; +import java.nio.file.PathMatcher; +import java.nio.file.WatchService; +import java.nio.file.attribute.UserPrincipalLookupService; +import java.util.Set; + +/** + * @author valerijf + */ +public class ContainerFileSystem extends FileSystem { + + private final ContainerFileSystemProvider containerFsProvider; + + public ContainerFileSystem(ContainerFileSystemProvider containerFsProvider) { + this.containerFsProvider = containerFsProvider; + } + + @Override + public ContainerFileSystemProvider provider() { + return containerFsProvider; + } + + @Override + public boolean isOpen() { + return true; + } + + @Override + public boolean isReadOnly() { + return false; + } + + @Override + public String getSeparator() { + return "/"; + } + + @Override + public Set<String> supportedFileAttributeViews() { + return Set.of("basic", "posix", "unix", "owner"); + } + + @Override + public UserPrincipalLookupService getUserPrincipalLookupService() { + return containerFsProvider.userPrincipalLookupService(); + } + + @Override + public ContainerPath getPath(String first, String... more) { + return ContainerPath.fromPathInContainer(this, Path.of(first, more)); + } + + @Override + public void close() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public Iterable<Path> getRootDirectories() { + throw new UnsupportedOperationException(); + } + + @Override + public Iterable<FileStore> getFileStores() { + throw new UnsupportedOperationException(); + } + + @Override + public PathMatcher getPathMatcher(String syntaxAndPattern) { + throw new UnsupportedOperationException(); + } + + @Override + public WatchService newWatchService() { + throw new UnsupportedOperationException(); + } + +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java new file mode 100644 index 00000000000..79214334c55 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java @@ -0,0 +1,265 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import java.io.IOException; +import java.net.URI; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.AccessMode; +import java.nio.file.CopyOption; +import java.nio.file.DirectoryStream; +import java.nio.file.FileStore; +import java.nio.file.FileSystem; +import java.nio.file.FileSystemAlreadyExistsException; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.ProviderMismatchException; +import java.nio.file.attribute.BasicFileAttributeView; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.UserPrincipal; +import java.nio.file.spi.FileSystemProvider; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.Set; + +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerAttributeViews.ContainerPosixFileAttributes; +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerAttributeViews.ContainerPosixFileAttributeView; +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.ContainerGroupPrincipal; +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.ContainerUserPrincipal; +import static com.yahoo.yolean.Exceptions.uncheck; + +/** + * @author valerijf + */ +public class ContainerFileSystemProvider extends FileSystemProvider { + + private final ContainerFileSystem containerFs; + private final ContainerUserPrincipalLookupService userPrincipalLookupService; + private final Path containerRootOnHost; + + + public ContainerFileSystemProvider(Path containerRootOnHost, int uidOffset, int gidOffset) { + this.containerFs = new ContainerFileSystem(this); + this.userPrincipalLookupService = new ContainerUserPrincipalLookupService( + containerRootOnHost.getFileSystem().getUserPrincipalLookupService(), uidOffset, gidOffset); + this.containerRootOnHost = containerRootOnHost; + } + + public Path containerRootOnHost() { + return containerRootOnHost; + } + + public ContainerUserPrincipalLookupService userPrincipalLookupService() { + return userPrincipalLookupService; + } + + @Override + public String getScheme() { + return "file"; + } + + @Override + public FileSystem newFileSystem(URI uri, Map<String, ?> env) { + throw new FileSystemAlreadyExistsException(); + } + + @Override + public ContainerFileSystem getFileSystem(URI uri) { + return containerFs; + } + + @Override + public Path getPath(URI uri) { + throw new UnsupportedOperationException(); + } + + @Override + public SeekableByteChannel newByteChannel(Path path, Set<? extends OpenOption> options, FileAttribute<?>... attrs) throws IOException { + Path pathOnHost = pathOnHost(path); + SeekableByteChannel seekableByteChannel = provider(pathOnHost).newByteChannel(pathOnHost, options, attrs); + fixOwnerToContainerRoot(toContainerPath(path)); + return seekableByteChannel; + } + + @Override + public DirectoryStream<Path> newDirectoryStream(Path dir, DirectoryStream.Filter<? super Path> filter) throws IOException { + Path pathOnHost = pathOnHost(dir); + return new ContainerDirectoryStream(provider(pathOnHost).newDirectoryStream(pathOnHost, filter)); + } + + @Override + public void createDirectory(Path dir, FileAttribute<?>... attrs) throws IOException { + Path pathOnHost = pathOnHost(dir); + provider(pathOnHost).createDirectory(pathOnHost); + fixOwnerToContainerRoot(toContainerPath(dir)); + } + + @Override + public void delete(Path path) throws IOException { + Path pathOnHost = pathOnHost(path); + provider(pathOnHost).delete(pathOnHost); + } + + @Override + public void copy(Path source, Path target, CopyOption... options) throws IOException { + // Only called when both 'source' and 'target' have 'this' as the FS provider + Path targetPathOnHost = pathOnHost(target); + provider(targetPathOnHost).copy(pathOnHost(source), targetPathOnHost, options); + fixOwnerToContainerRoot(toContainerPath(target)); + } + + @Override + public void move(Path source, Path target, CopyOption... options) throws IOException { + // Only called when both 'source' and 'target' have 'this' as the FS provider + Path targetPathOnHost = pathOnHost(target); + provider(targetPathOnHost).move(pathOnHost(source), targetPathOnHost, options); + fixOwnerToContainerRoot(toContainerPath(target)); + } + + @Override + public boolean isSameFile(Path path, Path path2) throws IOException { + // 'path' FS provider should be 'this' + if (path2 instanceof ContainerPath) + path2 = pathOnHost(path2); + Path pathOnHost = pathOnHost(path); + return provider(pathOnHost).isSameFile(pathOnHost, path2); + } + + @Override + public boolean isHidden(Path path) throws IOException { + Path pathOnHost = pathOnHost(path); + return provider(pathOnHost).isHidden(pathOnHost); + } + + @Override + public FileStore getFileStore(Path path) { + throw new UnsupportedOperationException(); + } + + @Override + public void checkAccess(Path path, AccessMode... modes) throws IOException { + Path pathOnHost = pathOnHost(path); + provider(pathOnHost).checkAccess(pathOnHost, modes); + } + + @Override + @SuppressWarnings("unchecked") + public <V extends FileAttributeView> V getFileAttributeView(Path path, Class<V> type, LinkOption... options) { + if (!type.isAssignableFrom(PosixFileAttributeView.class)) return null; + Path pathOnHost = pathOnHost(path); + FileSystemProvider provider = pathOnHost.getFileSystem().provider(); + if (type == BasicFileAttributeView.class) // Basic view doesnt have owner/group fields, forward to base FS provider + return provider.getFileAttributeView(pathOnHost, type, options); + + PosixFileAttributeView view = provider.getFileAttributeView(pathOnHost, PosixFileAttributeView.class, options); + return (V) new ContainerPosixFileAttributeView(view, + uncheck(() -> new ContainerPosixFileAttributes(readAttributes(path, "unix:*", options)))); + } + + @Override + @SuppressWarnings("unchecked") + public <A extends BasicFileAttributes> A readAttributes(Path path, Class<A> type, LinkOption... options) throws IOException { + if (!type.isAssignableFrom(PosixFileAttributes.class)) throw new UnsupportedOperationException(); + Path pathOnHost = pathOnHost(path); + if (type == BasicFileAttributes.class) + return pathOnHost.getFileSystem().provider().readAttributes(pathOnHost, type, options); + + // Non-basic requests need to be upgraded to unix:* to get owner,group,uid,gid fields, which are then re-mapped + return (A) new ContainerPosixFileAttributes(readAttributes(path, "unix:*", options)); + } + + @Override + public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) throws IOException { + Path pathOnHost = pathOnHost(path); + int index = attributes.indexOf(':'); + if (index < 0 || attributes.startsWith("basic:")) + return provider(pathOnHost).readAttributes(pathOnHost, attributes, options); + + Map<String, Object> attrs = new HashMap<>(provider(pathOnHost).readAttributes(pathOnHost, "unix:*", options)); + int uid = userPrincipalLookupService.hostUidToContainerUid((int) attrs.get("uid")); + int gid = userPrincipalLookupService.hostGidToContainerGid((int) attrs.get("gid")); + attrs.put("uid", uid); + attrs.put("gid", gid); + attrs.put("owner", new ContainerUserPrincipal(uid, (UserPrincipal) attrs.get("owner"))); + attrs.put("group", new ContainerGroupPrincipal(gid, (GroupPrincipal) attrs.get("group"))); + return attrs; + } + + @Override + public void setAttribute(Path path, String attribute, Object value, LinkOption... options) throws IOException { + Path pathOnHost = pathOnHost(path); + provider(pathOnHost).setAttribute(pathOnHost, attribute, fixAttributeValue(attribute, value), options); + } + + private Object fixAttributeValue(String attribute, Object value) { + int index = attribute.indexOf(':'); + if (index > 0) { + switch (attribute.substring(index + 1)) { + case "owner": return cast(value, ContainerUserPrincipal.class).baseFsPrincipal(); + case "group": return cast(value, ContainerGroupPrincipal.class).baseFsPrincipal(); + case "uid": return userPrincipalLookupService.containerUidToHostUid(cast(value, Integer.class)); + case "gid": return userPrincipalLookupService.containerGidToHostGid(cast(value, Integer.class)); + } + } // else basic file attribute + return value; + } + + private void fixOwnerToContainerRoot(ContainerPath path) throws IOException { + setAttribute(path, "unix:uid", 0); + setAttribute(path, "unix:gid", 0); + } + + private class ContainerDirectoryStream implements DirectoryStream<Path> { + private final DirectoryStream<Path> hostDirectoryStream; + + private ContainerDirectoryStream(DirectoryStream<Path> hostDirectoryStream) { + this.hostDirectoryStream = hostDirectoryStream; + } + + @Override + public Iterator<Path> iterator() { + Iterator<Path> hostPathIterator = hostDirectoryStream.iterator(); + return new Iterator<>() { + @Override + public boolean hasNext() { + return hostPathIterator.hasNext(); + } + + @Override + public Path next() { + Path pathOnHost = hostPathIterator.next(); + return ContainerPath.fromPathOnHost(containerFs, pathOnHost); + } + }; + } + + @Override + public void close() throws IOException { + hostDirectoryStream.close(); + } + } + + + static ContainerPath toContainerPath(Path path) { + return cast(path, ContainerPath.class); + } + + private static <T> T cast(Object value, Class<T> type) { + if (type.isInstance(value)) return type.cast(value); + throw new ProviderMismatchException("Expected " + type.getName() + ", was " + value.getClass().getName()); + } + + private static Path pathOnHost(Path path) { + return toContainerPath(path).pathOnHost(); + } + + private static FileSystemProvider provider(Path path) { + return path.getFileSystem().provider(); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java new file mode 100644 index 00000000000..e967806dc55 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPath.java @@ -0,0 +1,223 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import java.io.IOException; +import java.net.URI; +import java.nio.file.FileSystem; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.WatchEvent; +import java.nio.file.WatchKey; +import java.nio.file.WatchService; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerFileSystemProvider.toContainerPath; + +/** + * @author valerijf + */ +public class ContainerPath implements Path { + private final ContainerFileSystem containerFs; + private final Path pathOnHost; + private final String[] parts; + + private ContainerPath(ContainerFileSystem containerFs, Path pathOnHost, String[] parts) { + this.containerFs = Objects.requireNonNull(containerFs); + this.pathOnHost = Objects.requireNonNull(pathOnHost); + this.parts = Objects.requireNonNull(parts); + + if (!pathOnHost.isAbsolute()) + throw new IllegalArgumentException("Path host must be absolute: " + pathOnHost); + Path containerRootOnHost = containerFs.provider().containerRootOnHost(); + if (!pathOnHost.startsWith(containerRootOnHost)) + throw new IllegalArgumentException("Path on host (" + pathOnHost + ") must start with container root on host (" + containerRootOnHost + ")"); + } + + public Path pathOnHost() { + return pathOnHost; + } + + @Override + public FileSystem getFileSystem() { + return containerFs; + } + + @Override + public ContainerPath getRoot() { + return resolve(containerFs, new String[0], Path.of("/")); + } + + @Override + public Path getFileName() { + if (parts.length == 0) return null; + return Path.of(parts[parts.length - 1]); + } + + @Override + public ContainerPath getParent() { + if (parts.length == 0) return null; + return new ContainerPath(containerFs, pathOnHost.getParent(), Arrays.copyOf(parts, parts.length-1)); + } + + @Override + public int getNameCount() { + return parts.length; + } + + @Override + public Path getName(int index) { + return Path.of(parts[index]); + } + + @Override + public Path subpath(int beginIndex, int endIndex) { + if (beginIndex < 0 || beginIndex >= endIndex || endIndex > parts.length) + throw new IllegalArgumentException(); + if (endIndex - beginIndex == 1) return getName(beginIndex); + + String[] rest = new String[endIndex - beginIndex - 1]; + System.arraycopy(parts, beginIndex + 1, rest, 0, rest.length); + return Path.of(parts[beginIndex], rest); + } + + @Override + public ContainerPath resolve(Path other) { + return resolve(containerFs, parts, other); + } + + @Override + public ContainerPath resolveSibling(String other) { + return resolve(Path.of("..", other)); + } + + @Override + public boolean startsWith(Path other) { + if (other.getFileSystem() != containerFs) return false; + String[] otherParts = toContainerPath(other).parts; + if (parts.length < otherParts.length) return false; + + for (int i = 0; i < otherParts.length; i++) { + if ( ! parts[i].equals(otherParts[i])) return false; + } + return true; + } + + @Override + public boolean endsWith(Path other) { + int offset = parts.length - other.getNameCount(); + // If the other path is longer than this, or the other path is absolute and shorter than this + if (offset < 0 || (other.isAbsolute() && offset > 0)) return false; + + for (int i = 0; i < other.getNameCount(); i++) { + if ( ! parts[offset + i].equals(other.getName(i).toString())) return false; + } + return true; + } + + @Override + public boolean isAbsolute() { + // All container paths are normalized and absolute + return true; + } + + @Override + public ContainerPath normalize() { + // All container paths are normalized and absolute + return this; + } + + @Override + public ContainerPath toAbsolutePath() { + // All container paths are normalized and absolute + return this; + } + + @Override + public ContainerPath toRealPath(LinkOption... options) throws IOException { + Path realPathOnHost = pathOnHost.toRealPath(options); + if (realPathOnHost.equals(pathOnHost)) return this; + return fromPathOnHost(containerFs, realPathOnHost); + } + + @Override + public Path relativize(Path other) { + return pathOnHost.relativize(toContainerPath(other).pathOnHost); + } + + @Override + public URI toUri() { + throw new UnsupportedOperationException(); + } + + @Override + public WatchKey register(WatchService watcher, WatchEvent.Kind<?>[] events, WatchEvent.Modifier... modifiers) throws IOException { + return pathOnHost.register(watcher, events, modifiers); + } + + @Override + public int compareTo(Path other) { + return pathOnHost.compareTo(toContainerPath(other)); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ContainerPath paths = (ContainerPath) o; + return containerFs.equals(paths.containerFs) && pathOnHost.equals(paths.pathOnHost) && Arrays.equals(parts, paths.parts); + } + + @Override + public int hashCode() { + int result = Objects.hash(containerFs, pathOnHost); + result = 31 * result + Arrays.hashCode(parts); + return result; + } + + @Override + public String toString() { + return '/' + String.join("/", parts); + } + + private static ContainerPath resolve(ContainerFileSystem containerFs, String[] currentParts, Path other) { + List<String> parts = other.isAbsolute() ? new ArrayList<>() : new ArrayList<>(Arrays.asList(currentParts)); + for (int i = 0; i < other.getNameCount(); i++) { + String part = other.getName(i).toString(); + if (part.isEmpty() || part.equals(".")) continue; + if (part.equals("..")) { + if (!parts.isEmpty()) parts.remove(parts.size() - 1); + continue; + } + parts.add(part); + } + + return new ContainerPath(containerFs, + containerFs.provider().containerRootOnHost().resolve(String.join("/", parts)), + parts.toArray(String[]::new)); + } + + static ContainerPath fromPathInContainer(ContainerFileSystem containerFs, Path pathInContainer) { + if (!pathInContainer.isAbsolute()) + throw new IllegalArgumentException("Path in container must be absolute: " + pathInContainer); + return resolve(containerFs, new String[0], pathInContainer); + } + +static ContainerPath fromPathOnHost(ContainerFileSystem containerFs, Path pathOnHost) { + pathOnHost = pathOnHost.normalize(); + Path containerRootOnHost = containerFs.provider().containerRootOnHost(); + Path pathUnderContainerStorage = containerRootOnHost.relativize(pathOnHost); + + if (pathUnderContainerStorage.getNameCount() == 0 || pathUnderContainerStorage.getName(0).toString().isEmpty()) + return new ContainerPath(containerFs, pathOnHost, new String[0]); + if (pathUnderContainerStorage.getName(0).toString().equals("..")) + throw new IllegalArgumentException("Path " + pathOnHost + " is not under container root " + containerRootOnHost); + + List<String> parts = new ArrayList<>(); + for (int i = 0; i < pathUnderContainerStorage.getNameCount(); i++) + parts.add(pathUnderContainerStorage.getName(i).toString()); + return new ContainerPath(containerFs, pathOnHost, parts.toArray(String[]::new)); +} +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java new file mode 100644 index 00000000000..893e86ca239 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java @@ -0,0 +1,135 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import com.google.common.collect.ImmutableBiMap; + +import java.io.IOException; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.UserPrincipal; +import java.nio.file.attribute.UserPrincipalLookupService; +import java.nio.file.attribute.UserPrincipalNotFoundException; +import java.util.Objects; +import java.util.Optional; + +/** + * @author valerijf + */ +class ContainerUserPrincipalLookupService extends UserPrincipalLookupService { + + /** Total number of UID/GID that are mapped for each container */ + private static final int ID_RANGE = 1 << 16; + + /** + * IDs outside the ID range are translated to the overflow ID before being written to disk: + * https://github.com/torvalds/linux/blob/5bfc75d92efd494db37f5c4c173d3639d4772966/Documentation/admin-guide/sysctl/fs.rst#overflowgid--overflowuid */ + static final int OVERFLOW_ID = 65_534; + + private static final ImmutableBiMap<String, Integer> CONTAINER_IDS_BY_NAME = ImmutableBiMap.<String, Integer>builder() + .put("root", 0) + .put("vespa", 1000) + .build(); + + private final UserPrincipalLookupService baseFsUserPrincipalLookupService; + private final int uidOffset; + private final int gidOffset; + + ContainerUserPrincipalLookupService(UserPrincipalLookupService baseFsUserPrincipalLookupService, int uidOffset, int gidOffset) { + this.baseFsUserPrincipalLookupService = baseFsUserPrincipalLookupService; + this.uidOffset = uidOffset; + this.gidOffset = gidOffset; + } + + public int containerUidToHostUid(int containerUid) { return containerIdToHostId(containerUid, uidOffset); } + public int containerGidToHostGid(int containerGid) { return containerIdToHostId(containerGid, gidOffset); } + public int hostUidToContainerUid(int hostUid) { return hostIdToContainerId(hostUid, uidOffset); } + public int hostGidToContainerGid(int hostGid) { return hostIdToContainerId(hostGid, gidOffset); } + + @Override + public ContainerUserPrincipal lookupPrincipalByName(String name) throws IOException { + int containerUid = resolve(name); + String hostUid = String.valueOf(containerUidToHostUid(containerUid)); + return new ContainerUserPrincipal(containerUid, baseFsUserPrincipalLookupService.lookupPrincipalByName(hostUid)); + } + + @Override + public ContainerGroupPrincipal lookupPrincipalByGroupName(String group) throws IOException { + int containerGid = resolve(group); + String hostGid = String.valueOf(containerGidToHostGid(containerGid)); + return new ContainerGroupPrincipal(containerGid, baseFsUserPrincipalLookupService.lookupPrincipalByGroupName(hostGid)); + } + + private static int resolve(String name) throws UserPrincipalNotFoundException { + Integer id = CONTAINER_IDS_BY_NAME.get(name); + if (id != null) return id; + + try { + return Integer.parseInt(name); + } catch (NumberFormatException ignored) { + throw new UserPrincipalNotFoundException(name); + } + } + + private abstract static class NamedPrincipal implements UserPrincipal { + private final int id; + private final String name; + private final UserPrincipal baseFsPrincipal; + + private NamedPrincipal(int id, UserPrincipal baseFsPrincipal) { + this.id = id; + this.name = Optional.ofNullable(CONTAINER_IDS_BY_NAME.inverse().get(id)).orElseGet(() -> Integer.toString(id)); + this.baseFsPrincipal = Objects.requireNonNull(baseFsPrincipal); + } + + @Override + public final String getName() { + return name; + } + + public int id() { + return id; + } + + public UserPrincipal baseFsPrincipal() { + return baseFsPrincipal; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NamedPrincipal that = (NamedPrincipal) o; + return id == that.id && baseFsPrincipal.equals(that.baseFsPrincipal); + } + + @Override + public int hashCode() { + return Objects.hash(id, baseFsPrincipal); + } + + @Override + public String toString() { + return "{id=" + id + ", baseFsPrincipal=" + baseFsPrincipal + '}'; + } + } + + static final class ContainerUserPrincipal extends NamedPrincipal { + ContainerUserPrincipal(int id, UserPrincipal baseFsPrincipal) { super(id, baseFsPrincipal); } + } + + static final class ContainerGroupPrincipal extends NamedPrincipal implements GroupPrincipal { + ContainerGroupPrincipal(int id, GroupPrincipal baseFsPrincipal) { super(id, baseFsPrincipal); } + + @Override public GroupPrincipal baseFsPrincipal() { return (GroupPrincipal) super.baseFsPrincipal(); } + } + + private static int containerIdToHostId(int id, int idOffset) { + if (id < 0 || id > ID_RANGE) + throw new IllegalArgumentException("Invalid container id: " + id); + return idOffset + id; + } + + private static int hostIdToContainerId(int id, int idOffset) { + id = id - idOffset; + return id < 0 || id >= ID_RANGE ? OVERFLOW_ID : id; + } +} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducersTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducersTest.java new file mode 100644 index 00000000000..289f51ca867 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducersTest.java @@ -0,0 +1,29 @@ +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump;// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +import com.yahoo.yolean.concurrent.Sleeper; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * @author bjorncs + */ +class ArtifactProducersTest { + + @Test + void generates_exception_on_unknown_artifact() { + ArtifactProducers instance = ArtifactProducers.createDefault(Sleeper.NOOP); + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, () -> instance.resolve(List.of("unknown-artifact"))); + String expectedMsg = + "Invalid artifact type 'unknown-artifact'. Valid types are " + + "['jvm-heap-dump', 'jvm-jfr', 'jvm-jmap', 'jvm-jstack', 'jvm-jstat', 'perf-report', 'pmap', " + + "'vespa-log'] " + + "and valid aliases are " + + "['jvm-dump': ['jvm-heap-dump', 'jvm-jmap', 'jvm-jstack', 'jvm-jstat', 'vespa-log']]"; + assertEquals(expectedMsg, exception.getMessage()); + } + +}
\ No newline at end of file diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java index c8a67fcc8d8..0853223d142 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java @@ -1,4 +1,5 @@ -package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump;// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; import com.yahoo.yolean.concurrent.Sleeper; import com.yahoo.test.ManualClock; @@ -52,62 +53,68 @@ class VespaServiceDumperImplTest { void creates_valid_dump_id_from_dump_request() { long nowMillis = Instant.now().toEpochMilli(); ServiceDumpReport request = new ServiceDumpReport( - nowMillis, null, null, null, null, "default/container.3", null, null, List.of(JvmDumpProducer.NAME), null); + nowMillis, null, null, null, null, "default/container.3", null, null, List.of("perf-report"), null); String dumpId = VespaServiceDumperImpl.createDumpId(request); assertEquals("default-container-3-" + nowMillis, dumpId); } @Test - void generates_jvm_dump_from_request() throws IOException { + void invokes_perf_commands_when_generating_perf_report() { // Setup mocks ContainerOperations operations = mock(ContainerOperations.class); when(operations.executeCommandInContainerAsRoot(any(), any())) - .thenAnswer(invocation -> { - // Create dummy files to simulate vespa-jvm-dumper - Files.createFile(tmpDirectory.resolve("vespa-service-dump/" + JvmDumpProducer.NAME + "/heap.bin")); - Files.createFile(tmpDirectory.resolve("vespa-service-dump/" + JvmDumpProducer.NAME + "/jstack")); - return new CommandResult(null, 0, "result"); - }); + .thenReturn(new CommandResult(null, 0, "12345")) + .thenReturn(new CommandResult(null, 0, "")) + .thenReturn(new CommandResult(null, 0, "")); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); - NodeSpec initialSpec = createNodeSpecWithDumpRequest(nodeRepository, JvmDumpProducer.NAME, null); + NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, List.of("perf-report"), new ServiceDumpReport.DumpOptions(true, 45.0, null)); - // Create dumper and invoke tested method - VespaServiceDumper reporter = new VespaServiceDumperImpl(operations, syncClient, nodeRepository, clock, Sleeper.NOOP); - NodeAgentContextImpl context = new NodeAgentContextImpl.Builder(initialSpec) + VespaServiceDumper reporter = new VespaServiceDumperImpl( + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); + NodeAgentContextImpl context = new NodeAgentContextImpl.Builder(nodeSpec) .fileSystem(fileSystem) .build(); reporter.processServiceDumpRequest(context); - // Verify - String expectedJson = - "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000,\"completedAt\":1600001000000," + - "\"location\":\"s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/\"," + - "\"configId\":\"default/container.1\",\"artifacts\":[\"jvm-dump\"]}"; - assertReportEquals(nodeRepository, expectedJson); verify(operations).executeCommandInContainerAsRoot( - context, "/opt/vespa/bin/vespa-jvm-dumper", "default/container.1", "/opt/vespa/tmp/vespa-service-dump/jvm-dump"); + context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); + verify(operations).executeCommandInContainerAsRoot( + context, "perf", "record", "-g", "--output=/opt/vespa/tmp/vespa-service-dump/perf-record.bin", + "--pid=12345", "sleep", "45"); + verify(operations).executeCommandInContainerAsRoot( + context, "bash", "-c", "perf report --input=/opt/vespa/tmp/vespa-service-dump/perf-record.bin" + + " > /opt/vespa/tmp/vespa-service-dump/perf-report.txt"); + + String expectedJson = "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000,\"completedAt\":1600001000000," + + "\"location\":\"s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/\"," + + "\"configId\":\"default/container.1\",\"artifacts\":[\"perf-report\"]," + + "\"dumpOptions\":{\"callGraphRecording\":true,\"duration\":45.0}}"; + assertReportEquals(nodeRepository, expectedJson); + List<URI> expectedUris = List.of( - URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/jvm-dump/heap.bin.zst"), - URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/jvm-dump/jstack")); + URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/perf-record.bin.zst"), + URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/perf-report.txt")); assertSyncedFiles(context, syncClient, expectedUris); } @Test - void invokes_perf_commands_when_generating_perf_report() { + void invokes_jcmd_commands_when_creating_jfr_recording() { // Setup mocks ContainerOperations operations = mock(ContainerOperations.class); when(operations.executeCommandInContainerAsRoot(any(), any())) .thenReturn(new CommandResult(null, 0, "12345")) - .thenReturn(new CommandResult(null, 0, "")) - .thenReturn(new CommandResult(null, 0, "")); + .thenReturn(new CommandResult(null, 0, "ok")) + .thenReturn(new CommandResult(null, 0, "name=host-admin success")); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); - NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, PerfReportProducer.NAME, new ServiceDumpReport.DumpOptions(true, 45.0)); + NodeSpec nodeSpec = createNodeSpecWithDumpRequest( + nodeRepository, List.of("jvm-jfr"), new ServiceDumpReport.DumpOptions(null, null, null)); - VespaServiceDumper reporter = new VespaServiceDumperImpl(operations, syncClient, nodeRepository, clock, Sleeper.NOOP); + VespaServiceDumper reporter = new VespaServiceDumperImpl( + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); NodeAgentContextImpl context = new NodeAgentContextImpl.Builder(nodeSpec) .fileSystem(fileSystem) .build(); @@ -116,44 +123,57 @@ class VespaServiceDumperImplTest { verify(operations).executeCommandInContainerAsRoot( context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); verify(operations).executeCommandInContainerAsRoot( - context, "perf", "record", "-g", "--output=/opt/vespa/tmp/vespa-service-dump/perf-report/perf-record.bin", - "--pid=12345", "sleep", "45"); - verify(operations).executeCommandInContainerAsRoot( - context, "bash", "-c", "perf report --input=/opt/vespa/tmp/vespa-service-dump/perf-report/perf-record.bin" + - " > /opt/vespa/tmp/vespa-service-dump/perf-report/perf-report.txt"); + context, "jcmd", "12345", "JFR.start", "name=host-admin", "path-to-gc-roots=true", "settings=profile", + "filename=/opt/vespa/tmp/vespa-service-dump/recording.jfr", "duration=30s"); + verify(operations).executeCommandInContainerAsRoot(context, "jcmd", "12345", "JFR.check", "name=host-admin"); + + String expectedJson = "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000," + + "\"completedAt\":1600001000000," + + "\"location\":\"s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/\"," + + "\"configId\":\"default/container.1\",\"artifacts\":[\"jvm-jfr\"],\"dumpOptions\":{}}"; + assertReportEquals(nodeRepository, expectedJson); + + List<URI> expectedUris = List.of( + URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/recording.jfr.zst")); + assertSyncedFiles(context, syncClient, expectedUris); } @Test - void invokes_jcmd_commands_when_creating_jfr_recording() { + void handles_multiple_artifact_types() { // Setup mocks ContainerOperations operations = mock(ContainerOperations.class); when(operations.executeCommandInContainerAsRoot(any(), any())) + // For perf report: + .thenReturn(new CommandResult(null, 0, "12345")) + .thenReturn(new CommandResult(null, 0, "")) + .thenReturn(new CommandResult(null, 0, "")) + // For jfr recording: .thenReturn(new CommandResult(null, 0, "12345")) .thenReturn(new CommandResult(null, 0, "ok")) .thenReturn(new CommandResult(null, 0, "name=host-admin success")); SyncClient syncClient = createSyncClientMock(); NodeRepoMock nodeRepository = new NodeRepoMock(); ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L)); - NodeSpec nodeSpec = createNodeSpecWithDumpRequest( - nodeRepository, JavaFlightRecorder.NAME, new ServiceDumpReport.DumpOptions(null, null)); - - VespaServiceDumper reporter = new VespaServiceDumperImpl(operations, syncClient, nodeRepository, clock, Sleeper.NOOP); + NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, List.of("perf-report", "jvm-jfr"), + new ServiceDumpReport.DumpOptions(true, 20.0, null)); + VespaServiceDumper reporter = new VespaServiceDumperImpl( + ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock); NodeAgentContextImpl context = new NodeAgentContextImpl.Builder(nodeSpec) .fileSystem(fileSystem) .build(); reporter.processServiceDumpRequest(context); - verify(operations).executeCommandInContainerAsRoot( - context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); - verify(operations).executeCommandInContainerAsRoot( - context, "jcmd", "12345", "JFR.start", "name=host-admin", "path-to-gc-roots=true", "settings=profile", - "filename=/opt/vespa/tmp/vespa-service-dump/jfr-recording/recording.jfr", "duration=30s"); - verify(operations).executeCommandInContainerAsRoot(context, "jcmd", "12345", "JFR.check", "name=host-admin"); + List<URI> expectedUris = List.of( + URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/perf-record.bin.zst"), + URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/perf-report.txt"), + URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/recording.jfr.zst")); + assertSyncedFiles(context, syncClient, expectedUris); } - private static NodeSpec createNodeSpecWithDumpRequest(NodeRepoMock repository, String artifactName, ServiceDumpReport.DumpOptions options) { + private static NodeSpec createNodeSpecWithDumpRequest(NodeRepoMock repository, List<String> artifacts, + ServiceDumpReport.DumpOptions options) { ServiceDumpReport request = ServiceDumpReport.createRequestReport( - Instant.ofEpochMilli(1600000000000L), null, "default/container.1", List.of(artifactName), options); + Instant.ofEpochMilli(1600000000000L), null, "default/container.1", artifacts, options); NodeSpec spec = NodeSpec.Builder .testSpec(HOSTNAME, NodeState.active) .report(ServiceDumpReport.REPORT_ID, request.toJsonNode()) diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java index 4478a26396e..7130ac54430 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfoTest.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.maintenance.sync; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; @@ -64,7 +65,7 @@ public class SyncFileInfoTest { } private static void assertForLogFile(Path srcPath, String destination, SyncFileInfo.Compression compression, boolean rotatedOnly) { - Optional<SyncFileInfo> sfi = SyncFileInfo.forLogFile(nodeArchiveUri, srcPath, rotatedOnly); + Optional<SyncFileInfo> sfi = SyncFileInfo.forLogFile(nodeArchiveUri, srcPath, rotatedOnly, ApplicationId.defaultId()); assertEquals(destination, sfi.map(SyncFileInfo::destination).map(URI::toString).orElse(null)); assertEquals(compression, sfi.map(SyncFileInfo::uploadCompression).orElse(null)); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java new file mode 100644 index 00000000000..38c1e2720c3 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java @@ -0,0 +1,82 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.test.file.TestFileSystem; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.OVERFLOW_ID; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author valerijf + */ +class ContainerFileSystemTest { + + private final FileSystem fileSystem = TestFileSystem.create(); + private final UnixPath containerRootOnHost = new UnixPath(fileSystem.getPath("/data/storage/ctr1")); + private final ContainerFileSystem containerFs = new ContainerFileSystemProvider(containerRootOnHost.createDirectories().toPath(), 10_000, 11_000).getFileSystem(null); + + @Test + public void creates_files_and_directories_with_container_root_as_owner() throws IOException { + ContainerPath containerPath = ContainerPath.fromPathInContainer(containerFs, Path.of("/opt/vespa/logs/file")); + UnixPath unixPath = new UnixPath(containerPath).createParents().writeUtf8File("hello world"); + + for (ContainerPath p = containerPath; p.getParent() != null; p = p.getParent()) + assertOwnership(p, 0, 0, 10000, 11000); + + unixPath.setOwnerId(500).setGroupId(1000); + assertOwnership(containerPath, 500, 1000, 10500, 12000); + + UnixPath hostFile = new UnixPath(fileSystem.getPath("/file")).createNewFile(); + ContainerPath destination = ContainerPath.fromPathInContainer(containerFs, Path.of("/copy1")); + Files.copy(hostFile.toPath(), destination); + assertOwnership(destination, 0, 0, 10000, 11000); + } + + @Test + public void copy() throws IOException { + UnixPath hostFile = new UnixPath(fileSystem.getPath("/file")).createNewFile(); + ContainerPath destination = ContainerPath.fromPathInContainer(containerFs, Path.of("/dest")); + + // If file is copied to JimFS path, the UID/GIDs are not fixed + Files.copy(hostFile.toPath(), destination.pathOnHost()); + assertEquals(String.valueOf(OVERFLOW_ID), Files.getOwner(destination).getName()); + Files.delete(destination); + + Files.copy(hostFile.toPath(), destination); + assertOwnership(destination, 0, 0, 10000, 11000); + } + + @Test + public void move() throws IOException { + UnixPath hostFile = new UnixPath(fileSystem.getPath("/file")).createNewFile(); + ContainerPath destination = ContainerPath.fromPathInContainer(containerFs, Path.of("/dest")); + + // If file is moved to JimFS path, the UID/GIDs are not fixed + Files.move(hostFile.toPath(), destination.pathOnHost()); + assertEquals(String.valueOf(OVERFLOW_ID), Files.getOwner(destination).getName()); + Files.delete(destination); + + hostFile.createNewFile(); + Files.move(hostFile.toPath(), destination); + assertOwnership(destination, 0, 0, 10000, 11000); + } + + private static void assertOwnership(ContainerPath path, int contUid, int contGid, int hostUid, int hostGid) throws IOException { + assertOwnership(path, contUid, contGid); + assertOwnership(path.pathOnHost(), hostUid, hostGid); + } + + private static void assertOwnership(Path path, int uid, int gid) throws IOException { + Map<String, Object> attrs = Files.readAttributes(path, "unix:*"); + assertEquals(uid, attrs.get("uid")); + assertEquals(gid, attrs.get("gid")); + } +} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java new file mode 100644 index 00000000000..ead4ad7ecde --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java @@ -0,0 +1,112 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import com.yahoo.vespa.test.file.TestFileSystem; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath.fromPathInContainer; +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath.fromPathOnHost; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; + +/** + * @author valerijf + */ +class ContainerPathTest { + + private final FileSystem baseFs = TestFileSystem.create(); + private final ContainerFileSystem containerFs = new ContainerFileSystemProvider(baseFs.getPath("/data/storage/ctr1"), 0, 0).getFileSystem(null); + + @Test + public void create_new_container_path() { + ContainerPath path = fromPathInContainer(containerFs, Path.of("/opt/vespa//logs/./file")); + assertPaths(path, "/data/storage/ctr1/opt/vespa/logs/file", "/opt/vespa/logs/file"); + + path = fromPathOnHost(containerFs, baseFs.getPath("/data/storage/ctr1/opt/vespa/logs/file")); + assertPaths(path, "/data/storage/ctr1/opt/vespa/logs/file", "/opt/vespa/logs/file"); + + path = fromPathOnHost(containerFs, baseFs.getPath("/data/storage/ctr2/..////./ctr1/./opt")); + assertPaths(path, "/data/storage/ctr1/opt", "/opt"); + + assertThrows(() -> fromPathInContainer(containerFs, Path.of("relative/path")), "Path in container must be absolute: relative/path"); + assertThrows(() -> fromPathOnHost(containerFs, baseFs.getPath("relative/path")), "Paths have different roots: /data/storage/ctr1, relative/path"); + assertThrows(() -> fromPathOnHost(containerFs, baseFs.getPath("/data/storage/ctr2")), "Path /data/storage/ctr2 is not under container root /data/storage/ctr1"); + assertThrows(() -> fromPathOnHost(containerFs, baseFs.getPath("/data/storage/ctr1/../ctr2")), "Path /data/storage/ctr2 is not under container root /data/storage/ctr1"); + } + + @Test + public void container_path_operations() { + ContainerPath path = fromPathInContainer(containerFs, Path.of("/opt/vespa/logs/file")); + ContainerPath parent = path.getParent(); + assertPaths(path.getRoot(), "/data/storage/ctr1", "/"); + assertPaths(parent, "/data/storage/ctr1/opt/vespa/logs", "/opt/vespa/logs"); + assertNull(path.getRoot().getParent()); + + assertEquals(Path.of("file"), path.getFileName()); + assertEquals(Path.of("logs"), path.getName(2)); + assertEquals(4, path.getNameCount()); + assertEquals(Path.of("vespa/logs"), path.subpath(1, 3)); + + assertTrue(path.startsWith(path)); + assertTrue(path.startsWith(parent)); + assertFalse(parent.startsWith(path)); + assertFalse(path.startsWith(Path.of(path.toString()))); + + assertTrue(path.endsWith(Path.of(path.toString()))); + assertTrue(path.endsWith(Path.of("logs/file"))); + assertFalse(path.endsWith(Path.of("/logs/file"))); + } + + @Test + public void resolution() { + ContainerPath path = fromPathInContainer(containerFs, Path.of("/opt/vespa/logs")); + assertPaths(path.resolve(Path.of("/root")), "/data/storage/ctr1/root", "/root"); + assertPaths(path.resolve(Path.of("relative")), "/data/storage/ctr1/opt/vespa/logs/relative", "/opt/vespa/logs/relative"); + assertPaths(path.resolve(Path.of("/../../../dir2/../../../dir2")), "/data/storage/ctr1/dir2", "/dir2"); + assertPaths(path.resolve(Path.of("/some/././///path")), "/data/storage/ctr1/some/path", "/some/path"); + + assertPaths(path.resolve(Path.of("../dir")), "/data/storage/ctr1/opt/vespa/dir", "/opt/vespa/dir"); + assertEquals(path.resolve(Path.of("../dir")), path.resolveSibling("dir")); + } + + @Test + public void resolves_real_paths() throws IOException { + ContainerPath path = fromPathInContainer(containerFs, Path.of("/opt/vespa/logs")); + Files.createDirectories(path.pathOnHost().getParent()); + + Files.createFile(baseFs.getPath("/data/storage/ctr1/opt/vespa/target1")); + Files.createSymbolicLink(path.pathOnHost(), path.pathOnHost().resolveSibling("target1")); + assertPaths(path.toRealPath(LinkOption.NOFOLLOW_LINKS), "/data/storage/ctr1/opt/vespa/logs", "/opt/vespa/logs"); + assertPaths(path.toRealPath(), "/data/storage/ctr1/opt/vespa/target1", "/opt/vespa/target1"); + + Files.delete(path.pathOnHost()); + Files.createFile(baseFs.getPath("/data/storage/ctr1/opt/target2")); + Files.createSymbolicLink(path.pathOnHost(), baseFs.getPath("../target2")); + assertPaths(path.toRealPath(), "/data/storage/ctr1/opt/target2", "/opt/target2"); + + Files.delete(path.pathOnHost()); + Files.createFile(baseFs.getPath("/data/storage/ctr2")); + Files.createSymbolicLink(path.pathOnHost(), path.getRoot().pathOnHost().resolveSibling("ctr2")); + assertThrows(path::toRealPath, "Path /data/storage/ctr2 is not under container root /data/storage/ctr1"); + } + + private static void assertPaths(ContainerPath actual, String expectedPathOnHost, String expectedPathInContainer) { + assertEquals(expectedPathOnHost, actual.pathOnHost().toString()); + assertEquals(expectedPathInContainer, actual.toString()); + } + + private static void assertThrows(Executable executable, String expectedMsg) { + String actualMsg = Assertions.assertThrows(IllegalArgumentException.class, executable).getMessage(); + assertEquals(expectedMsg, actualMsg); + } +}
\ No newline at end of file diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.java new file mode 100644 index 00000000000..a459c24049e --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.java @@ -0,0 +1,51 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import com.yahoo.vespa.test.file.TestFileSystem; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.attribute.UserPrincipalNotFoundException; + +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.ContainerUserPrincipal; +import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.ContainerGroupPrincipal; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * @author valerijf + */ +class ContainerUserPrincipalLookupServiceTest { + + private final ContainerUserPrincipalLookupService userPrincipalLookupService = + new ContainerUserPrincipalLookupService(TestFileSystem.create().getUserPrincipalLookupService(), 1000, 2000); + + @Test + public void correctly_resolves_ids() throws IOException { + ContainerUserPrincipal user = userPrincipalLookupService.lookupPrincipalByName("1000"); + assertEquals("vespa", user.getName()); + assertEquals("2000", user.baseFsPrincipal().getName()); + assertEquals(user, userPrincipalLookupService.lookupPrincipalByName("vespa")); + + ContainerGroupPrincipal group = userPrincipalLookupService.lookupPrincipalByGroupName("1000"); + assertEquals("vespa", group.getName()); + assertEquals("3000", group.baseFsPrincipal().getName()); + assertEquals(group, userPrincipalLookupService.lookupPrincipalByGroupName("vespa")); + + assertThrows(UserPrincipalNotFoundException.class, () -> userPrincipalLookupService.lookupPrincipalByName("test")); + } + + @Test + public void translates_between_ids() { + assertEquals(1001, userPrincipalLookupService.containerUidToHostUid(1)); + assertEquals(2001, userPrincipalLookupService.containerGidToHostGid(1)); + assertEquals(1, userPrincipalLookupService.hostUidToContainerUid(1001)); + assertEquals(1, userPrincipalLookupService.hostGidToContainerGid(2001)); + + assertEquals(65_534, userPrincipalLookupService.hostUidToContainerUid(1)); + assertEquals(65_534, userPrincipalLookupService.hostUidToContainerUid(999999)); + + assertThrows(IllegalArgumentException.class, () -> userPrincipalLookupService.containerUidToHostUid(-1)); + assertThrows(IllegalArgumentException.class, () -> userPrincipalLookupService.containerUidToHostUid(70_000)); + } +}
\ No newline at end of file diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index 9cffc361444..ff696e727fa 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.TransientException; +import com.yahoo.config.provision.exception.ActivationConflictException; import com.yahoo.jdisc.Metric; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; @@ -95,7 +96,7 @@ class MaintenanceDeployment implements Closeable { if ( ! isValid()) return Optional.empty(); try { return Optional.of(step.get()); - } catch (TransientException e) { + } catch (TransientException | ActivationConflictException e) { metric.add("maintenanceDeployment.transientFailure", 1, metric.createContext(Map.of())); log.log(Level.INFO, "Failed to maintenance deploy " + application + " with a transient error: " + Exceptions.toMessageString(e)); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index c3d15cbec2c..8842f426a3b 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -12,6 +12,7 @@ #include <vespa/searchlib/common/geo_location_parser.h> #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <vespa/searchlib/queryeval/intermediate_blueprints.h> +#include <vespa/vespalib/util/issue.h> #include <vespa/log/log.h> LOG_SETUP(".proton.matching.query"); @@ -38,6 +39,7 @@ using search::queryeval::IRequestContext; using search::queryeval::SearchIterator; using search::query::LocationTerm; using vespalib::string; +using vespalib::Issue; using std::vector; namespace proton::matching { @@ -89,7 +91,7 @@ GeoLocationSpec parse_location_string(string str) { auto attr_name = PositionDataType::getZCurveFieldName(parser.getFieldName()); return GeoLocationSpec{attr_name, parser.getGeoLocation()}; } else { - LOG(warning, "Location parse error (location: '%s'): %s", str.c_str(), parser.getParseError()); + Issue::report("Location parse error (location: '%s'): %s", str.c_str(), parser.getParseError()); } return empty; } @@ -175,7 +177,7 @@ Query::buildTree(vespalib::stringref stack, const string &location, _query_tree->accept(resolve_visitor); return true; } else { - // TODO(havardpe): log warning or pass message upwards + Issue::report("invalid query"); return false; } } diff --git a/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp b/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp index f2fd9ad52d6..348331665a0 100644 --- a/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp +++ b/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/stllike/string.h> #include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/util/issue.h> #include <vespa/searchlib/features/rankingexpressionfeature.h> #include <vespa/searchlib/features/onnx_feature.h> #include <vespa/searchlib/fef/blueprintfactory.h> @@ -18,6 +19,7 @@ using namespace search::fef::test; using namespace search::features; using vespalib::make_string_short::fmt; using vespalib::eval::TensorSpec; +using vespalib::Issue; std::string get_source_dir() { const char *dir = getenv("SOURCE_DIRECTORY"); @@ -153,13 +155,25 @@ TEST_F(OnnxFeatureTest, fragile_model_can_be_evaluated) { EXPECT_EQ(get(3), TensorSpec::from_expr("tensor<float>(d0[2]):[6,15]")); } +struct MyIssues : Issue::Handler { + std::vector<vespalib::string> list; + Issue::Binding capture; + MyIssues() : list(), capture(Issue::listen(*this)) {} + void handle(const Issue &issue) override { list.push_back(issue.message()); } +}; + TEST_F(OnnxFeatureTest, broken_model_evaluates_to_all_zeros) { add_expr("in1", "tensor<float>(x[2]):[docid,5]"); add_expr("in2", "tensor<float>(x[3]):[docid,10,31515]"); add_onnx(OnnxModel("fragile", fragile_model).dry_run_on_setup(false)); EXPECT_TRUE(try_compile(onnx_feature("fragile"))); + MyIssues my_issues; + EXPECT_EQ(my_issues.list.size(), 0); EXPECT_EQ(get(1), TensorSpec::from_expr("tensor<float>(d0[2]):[0,0]")); + EXPECT_EQ(my_issues.list.size(), 1); EXPECT_EQ(get(3), TensorSpec::from_expr("tensor<float>(d0[2]):[0,0]")); + ASSERT_EQ(my_issues.list.size(), 2); + EXPECT_EQ(my_issues.list[0], my_issues.list[1]); } TEST_F(OnnxFeatureTest, broken_model_fails_with_dry_run) { diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index 14070388f7b..213bbfc3493 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -620,7 +620,7 @@ public: void visitPredicate(PredicateQuery &query) { const PredicateAttribute *attr = dynamic_cast<const PredicateAttribute *>(&_attr); if (!attr) { - LOG(warning, "Trying to apply a PredicateQuery node to a non-predicate attribute."); + Issue::report("Trying to apply a PredicateQuery node to a non-predicate attribute."); setResult(std::make_unique<queryeval::EmptyBlueprint>(_field)); } else { setResult(std::make_unique<PredicateBlueprint>( _field, *attr, query)); @@ -724,8 +724,8 @@ public: } } void fail_nearest_neighbor_term(query::NearestNeighborTerm&n, const vespalib::string& error_msg) { - LOG(warning, "NearestNeighborTerm(%s, %s): %s. Returning empty blueprint", - _field.getName().c_str(), n.get_query_tensor_name().c_str(), error_msg.c_str()); + Issue::report("NearestNeighborTerm(%s, %s): %s. Returning empty blueprint", + _field.getName().c_str(), n.get_query_tensor_name().c_str(), error_msg.c_str()); setResult(std::make_unique<queryeval::EmptyBlueprint>(_field)); } void visit(query::NearestNeighborTerm &n) override { @@ -800,6 +800,7 @@ AttributeBlueprintFactory::createBlueprint(const IRequestContext & requestContex { const IAttributeVector *attr(requestContext.getAttribute(field.getName())); if (attr == nullptr) { + Issue::report("attribute not found: %s", field.getName().c_str()); return std::make_unique<queryeval::EmptyBlueprint>(field); } try { diff --git a/searchlib/src/vespa/searchlib/features/onnx_feature.cpp b/searchlib/src/vespa/searchlib/features/onnx_feature.cpp index 291e3daf602..dd0215e1d53 100644 --- a/searchlib/src/vespa/searchlib/features/onnx_feature.cpp +++ b/searchlib/src/vespa/searchlib/features/onnx_feature.cpp @@ -10,6 +10,7 @@ #include <vespa/eval/eval/value_codec.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/stash.h> +#include <vespa/vespalib/util/issue.h> #include <vespa/log/log.h> LOG_SETUP(".features.onnx_feature"); @@ -28,6 +29,7 @@ using vespalib::eval::FastValueBuilderFactory; using vespalib::eval::value_from_spec; using vespalib::make_string_short::fmt; using vespalib::eval::Onnx; +using vespalib::Issue; namespace search::features { @@ -92,7 +94,7 @@ public: try { _eval_context.eval(); } catch (const Ort::Exception &ex) { - LOG(warning, "onnx model evaluation failed: %s", ex.what()); + Issue::report("onnx model evaluation failed: %s", ex.what()); _eval_context.clear_results(); } } diff --git a/testutil/src/main/java/com/yahoo/vespa/test/file/UnixUidGidAttributeProvider.java b/testutil/src/main/java/com/yahoo/vespa/test/file/UnixUidGidAttributeProvider.java index 2b1b0231b4f..13c51851540 100644 --- a/testutil/src/main/java/com/yahoo/vespa/test/file/UnixUidGidAttributeProvider.java +++ b/testutil/src/main/java/com/yahoo/vespa/test/file/UnixUidGidAttributeProvider.java @@ -72,7 +72,8 @@ public class UnixUidGidAttributeProvider extends AttributeProvider { } private int getUniqueId(UserPrincipal user) { - return idCache.computeIfAbsent(user, id -> maybeNumber(id.getName()).orElseGet(uidGenerator::incrementAndGet)); + return maybeNumber(user.getName()) + .orElseGet(() -> idCache.computeIfAbsent(user, id -> uidGenerator.incrementAndGet())); } @SuppressWarnings("unchecked") @@ -106,6 +107,14 @@ public class UnixUidGidAttributeProvider extends AttributeProvider { @Override public void set(File file, String view, String attribute, Object value, boolean create) { + switch (attribute) { + case "uid": + file.setAttribute("owner", "owner", new BasicUserPrincipal(String.valueOf(value))); + return; + case "gid": + file.setAttribute("posix", "group", new BasicGroupPrincipal(String.valueOf(value))); + return; + } throw unsettable(view, attribute); } @@ -158,4 +167,16 @@ public class UnixUidGidAttributeProvider extends AttributeProvider { return Optional.empty(); } } + + private static class BasicUserPrincipal implements UserPrincipal { + private final String name; + private BasicUserPrincipal(String name) { this.name = name; } + + @Override public String getName() { return name; } + @Override public String toString() { return name; } + } + + private static class BasicGroupPrincipal extends BasicUserPrincipal implements GroupPrincipal { + private BasicGroupPrincipal(String name) { super(name); } + } } diff --git a/vespalib/src/vespa/vespalib/util/issue.cpp b/vespalib/src/vespa/vespalib/util/issue.cpp index d60423bfa4d..cfbadadd745 100644 --- a/vespalib/src/vespa/vespalib/util/issue.cpp +++ b/vespalib/src/vespa/vespalib/util/issue.cpp @@ -14,7 +14,7 @@ using Link = Issue::Binding::Link; struct LogIssues : Issue::Handler { void handle(const Issue &issue) override { - LOG(warning, "unhandled issue: %s", issue.message().c_str()); + LOG(warning, "%s", issue.message().c_str()); } }; |