Merge pull request #16540 from vespa-engine/freva/sync-logs-s3

Sync tenant node logs to S3 

5 files changed, 237 insertions, 214 deletions

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 a5efec1dcb7..c30add928ba 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,8 +3,13 @@ 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.flags.FetchVector;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.StringFlag;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.node.admin.component.TaskContext;
@@ -13,6 +18,8 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.disk.CoredumpCleanupRule;
 import com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanup;
 import com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanupRule;
 import com.yahoo.vespa.hosted.node.admin.maintenance.disk.LinearCleanupRule;
+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.nodeadmin.ConvergenceException;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentTask;
@@ -31,6 +38,7 @@ import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -39,6 +47,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanupRule.Priority;
 import static com.yahoo.yolean.Exceptions.uncheck;
@@ -53,9 +62,11 @@ public class StorageMaintainer {
 
     private final Terminal terminal;
     private final CoredumpHandler coredumpHandler;
-    private final Path archiveContainerStoragePath;
     private final DiskCleanup diskCleanup;
+    private final SyncClient syncClient;
     private final Clock clock;
+    private final Path archiveContainerStoragePath;
+    private final StringFlag syncBucketNameFlag;
 
     // We cache disk usage to avoid doing expensive disk operations so often
     private final Cache<ContainerName, DiskSize> diskUsage = CacheBuilder.newBuilder()
@@ -63,16 +74,34 @@ public class StorageMaintainer {
             .expireAfterWrite(5, TimeUnit.MINUTES)
             .build();
 
-    public StorageMaintainer(Terminal terminal, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath) {
-        this(terminal, coredumpHandler, archiveContainerStoragePath, new DiskCleanup(), Clock.systemUTC());
-    }
-
-    public StorageMaintainer(Terminal terminal, CoredumpHandler coredumpHandler, Path archiveContainerStoragePath, DiskCleanup diskCleanup, Clock clock) {
+    public StorageMaintainer(Terminal terminal, CoredumpHandler coredumpHandler, DiskCleanup diskCleanup,
+                             SyncClient syncClient, Clock clock, Path archiveContainerStoragePath, FlagSource flagSource) {
         this.terminal = terminal;
         this.coredumpHandler = coredumpHandler;
-        this.archiveContainerStoragePath = archiveContainerStoragePath;
         this.diskCleanup = diskCleanup;
+        this.syncClient = syncClient;
         this.clock = clock;
+        this.archiveContainerStoragePath = archiveContainerStoragePath;
+        this.syncBucketNameFlag = Flags.SYNC_HOST_LOGS_TO_S3_BUCKET.bindTo(flagSource);
+    }
+
+    public boolean syncLogs(NodeAgentContext context) {
+        Optional<ApplicationId> app = context.node().owner();
+        if (app.isEmpty()) return false;
+        String bucketName = syncBucketNameFlag
+                .with(FetchVector.Dimension.NODE_TYPE, NodeType.tenant.name())
+                .with(FetchVector.Dimension.APPLICATION_ID, app.get().serializedForm())
+                .value();
+        if (bucketName.isBlank()) return false;
+
+        List<SyncFileInfo> syncFileInfos = FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa"))
+                .maxDepth(2)
+                .stream()
+                .sorted(Comparator.comparing(FileFinder.FileAttributes::lastModifiedTime))
+                .flatMap(fa -> SyncFileInfo.tenantLog(bucketName, app.get(), context.hostname(), fa.path()).stream())
+                .collect(Collectors.toList());
+
+        return syncClient.sync(context, syncFileInfos, 1);
     }
 
     public Optional<DiskSize> diskUsageFor(NodeAgentContext context) {
@@ -127,22 +156,20 @@ public class StorageMaintainer {
         Instant start = clock.instant();
         double oneMonthSeconds = Duration.ofDays(30).getSeconds();
         Function<Instant, Double> monthNormalizer = instant -> Duration.between(instant, start).getSeconds() / oneMonthSeconds;
-        Function<String, Path> pathOnHostUnderContainerVespaHome = path ->
-                context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome(path));
         List<DiskCleanupRule> rules = new ArrayList<>();
 
-        rules.add(CoredumpCleanupRule.forContainer(pathOnHostUnderContainerVespaHome.apply("var/crash")));
+        rules.add(CoredumpCleanupRule.forContainer(pathOnHostUnderContainerVespaHome(context, "var/crash")));
 
         if (context.node().membership().map(m -> m.type().isContainer()).orElse(false))
-            rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome.apply("logs/vespa/qrs")).list(),
+            rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa/qrs")).list(),
                     fa -> monthNormalizer.apply(fa.lastModifiedTime()), Priority.LOWEST, Priority.HIGHEST));
 
         if (context.nodeType() == NodeType.tenant && context.node().membership().map(m -> m.type().isAdmin()).orElse(false))
-            rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome.apply("logs/vespa/logarchive")).list(),
+            rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa/logarchive")).list(),
                     fa -> monthNormalizer.apply(fa.lastModifiedTime()), Priority.LOWEST, Priority.HIGHEST));
 
         if (context.nodeType() == NodeType.proxy)
-            rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome.apply("logs/nginx")).list(),
+            rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/nginx")).list(),
                     fa -> monthNormalizer.apply(fa.lastModifiedTime()), Priority.LOWEST, Priority.MEDIUM));
 
         return rules;
@@ -212,4 +239,8 @@ public class StorageMaintainer {
                         .orElse("<none>")
                 );
     }
+
+    private static Path pathOnHostUnderContainerVespaHome(NodeAgentContext context, String path) {
+        return context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome(path));
+    }
 }
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 3b88d28613c..ad8782704dc 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
@@ -4,11 +4,9 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.sync;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Optional;
 
 /**
  * @author freva
@@ -18,52 +16,61 @@ public class SyncFileInfo {
     private final String bucketName;
     private final Path srcPath;
     private final Path destPath;
-    private final boolean compressWithZstd;
+    private final Compression uploadCompression;
 
-    private SyncFileInfo(String bucketName, Path srcPath, Path destPath, boolean compressWithZstd) {
+    private SyncFileInfo(String bucketName, Path srcPath, Path destPath, Compression uploadCompression) {
         this.bucketName = bucketName;
         this.srcPath = srcPath;
         this.destPath = destPath;
-        this.compressWithZstd = compressWithZstd;
+        this.uploadCompression = uploadCompression;
     }
 
     public String bucketName() {
         return bucketName;
     }
 
+    /** Source path of the file to sync */
     public Path srcPath() {
         return srcPath;
     }
 
+    /** Remote path to store the file at */
     public Path destPath() {
         return destPath;
     }
 
-    public InputStream inputStream() throws IOException {
-        InputStream is = Files.newInputStream(srcPath);
-        if (compressWithZstd) return new ZstdCompressingInputStream(is, 4 << 20);
-        return is;
+    /** Compression algorithm to use when uploading the file */
+    public Compression uploadCompression() {
+        return uploadCompression;
     }
 
+    public static Optional<SyncFileInfo> tenantLog(String bucketName, ApplicationId applicationId, HostName hostName, Path logFile) {
+        String filename = logFile.getFileName().toString();
+        Compression compression = Compression.NONE;
+        String dir = null;
+
+        if (filename.startsWith("vespa.log-")) {
+            dir = "logs/vespa";
+            compression = Compression.ZSTD;
+        } else if (filename.endsWith(".zst")) {
+            if (filename.startsWith("JsonAccessLog.") || filename.startsWith("access"))
+                dir = "logs/access";
+            else if (filename.startsWith("ConnectionLog."))
+                dir = "logs/connection";
+        }
 
-    public static SyncFileInfo tenantVespaLog(String bucketName, ApplicationId applicationId, HostName hostName, Path vespaLogFile) {
-        return new SyncFileInfo(bucketName, vespaLogFile, destination(applicationId, hostName, "logs/vespa", vespaLogFile, ".zst"), true);
-    }
-
-    public static SyncFileInfo tenantAccessLog(String bucketName, ApplicationId applicationId, HostName hostName, Path accessLogFile) {
-        return new SyncFileInfo(bucketName, accessLogFile, destination(applicationId, hostName, "logs/access", accessLogFile, null), false);
+        if (dir == null) return Optional.empty();
+        return Optional.of(new SyncFileInfo(
+                bucketName, logFile, destination(applicationId, hostName, dir, logFile, compression), compression));
     }
 
     public static SyncFileInfo infrastructureVespaLog(String bucketName, HostName hostName, Path vespaLogFile) {
-        return new SyncFileInfo(bucketName, vespaLogFile, destination(null, hostName, "logs/vespa", vespaLogFile, ".zst"), true);
-    }
-
-    public static SyncFileInfo infrastructureAccessLog(String bucketName, HostName hostName, Path accessLogFile) {
-        return new SyncFileInfo(bucketName, accessLogFile, destination(null, hostName, "logs/access", accessLogFile, null), false);
+        Compression compression = Compression.ZSTD;
+        return new SyncFileInfo(bucketName, vespaLogFile, destination(null, hostName, "logs/vespa", vespaLogFile, compression), compression);
     }
 
-    private static Path destination(ApplicationId app, HostName hostName, String dir, Path filename, String extension) {
-        StringBuilder sb = new StringBuilder(100).append('/');
+    private static Path destination(ApplicationId app, HostName hostName, String dir, Path filename, Compression uploadCompression) {
+        StringBuilder sb = new StringBuilder(100);
 
         if (app == null) sb.append("infrastructure");
         else sb.append(app.tenant().value()).append('.').append(app.application().value()).append('.').append(app.instance().value());
@@ -76,8 +83,17 @@ public class SyncFileInfo {
 
         sb.append('/').append(dir).append('/').append(filename.getFileName().toString());
 
-        if (extension != null) sb.append(extension);
+        if (uploadCompression.extension != null) sb.append(uploadCompression.extension);
 
         return Paths.get(sb.toString());
     }
+
+    public enum Compression {
+        NONE(null), ZSTD(".zst");
+
+        private final String extension;
+        Compression(String extension) {
+            this.extension = extension;
+        }
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 24928b4c8a6..4bc6fae4e56 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -499,6 +499,7 @@ public class NodeAgentImpl implements NodeAgent {
                     orchestrator.resume(context.hostname().value());
                     suspendedInOrchestrator = false;
                 }
+                storageMaintainer.syncLogs(context);
                 break;
             case provisioned:
                 nodeRepository.setNodeState(context.hostname().value(), NodeState.dirty);
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
index cb615a94615..a17ffadcb45 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
@@ -3,8 +3,11 @@ package com.yahoo.vespa.hosted.node.admin.maintenance;
 
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.test.ManualClock;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
+import com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoredumpHandler;
 import com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanup;
+import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncClient;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder;
@@ -13,8 +16,6 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
 import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.After;
 import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
 
 import java.io.IOException;
 import java.nio.file.FileSystem;
@@ -38,150 +39,141 @@ import static org.mockito.Mockito.verifyNoInteractions;
 /**
  * @author dybis
  */
-@RunWith(Enclosed.class)
 public class StorageMaintainerTest {
 
-    public static class DiskUsageTests {
-
-        private final TestTerminal terminal = new TestTerminal();
-
-        @Test
-        public void testDiskUsed() throws IOException {
-            StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, null, null);
-            FileSystem fileSystem = TestFileSystem.create();
-            NodeAgentContext context = new NodeAgentContextImpl.Builder("host-1.domain.tld").fileSystem(fileSystem).build();
-            Files.createDirectories(context.pathOnHostFromPathInNode("/"));
+    private final TestTerminal terminal = new TestTerminal();
+    private final CoredumpHandler coredumpHandler = mock(CoredumpHandler.class);
+    private final DiskCleanup diskCleanup = mock(DiskCleanup.class);
+    private final SyncClient syncClient = mock(SyncClient.class);
+    private final ManualClock clock = new ManualClock(Instant.ofEpochSecond(1234567890));
+    private final InMemoryFlagSource flagSource = new InMemoryFlagSource();
+    private final FileSystem fileSystem = TestFileSystem.create();
+    private final StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, coredumpHandler, diskCleanup, syncClient, clock,
+            fileSystem.getPath("/home/docker/container-storage/container-archive"), flagSource);
+
+    @Test
+    public void testDiskUsed() throws IOException {
+        NodeAgentContext context = new NodeAgentContextImpl.Builder("host-1.domain.tld").fileSystem(fileSystem).build();
+        Files.createDirectories(context.pathOnHostFromPathInNode("/"));
+
+        terminal.expectCommand("du -xsk /home/docker/container-storage/host-1 2>&1", 0, "321\t/home/docker/container-storage/host-1/");
+        assertEquals(Optional.of(DiskSize.of(328_704)), storageMaintainer.diskUsageFor(context));
+
+        // Value should still be cached, no new execution against the terminal
+        assertEquals(Optional.of(DiskSize.of(328_704)), storageMaintainer.diskUsageFor(context));
+    }
 
-            terminal.expectCommand("du -xsk /home/docker/container-storage/host-1 2>&1", 0, "321\t/home/docker/container-storage/host-1/");
-            assertEquals(Optional.of(DiskSize.of(328_704)), storageMaintainer.diskUsageFor(context));
+    @Test
+    public void testNonExistingDiskUsed() {
+        DiskSize size = storageMaintainer.getDiskUsed(null, Paths.get("/fake/path"));
+        assertEquals(DiskSize.ZERO, size);
+    }
 
-            // Value should still be cached, no new execution against the terminal
-            assertEquals(Optional.of(DiskSize.of(328_704)), storageMaintainer.diskUsageFor(context));
-        }
+    @Test
+    public void archive_container_data_test() throws IOException {
+        // Create some files in containers
+        NodeAgentContext context1 = createNodeAgentContextAndContainerStorage(fileSystem, "container-1");
+        createNodeAgentContextAndContainerStorage(fileSystem, "container-2");
+
+        Path pathToArchiveDir = fileSystem.getPath("/home/docker/container-storage/container-archive");
+        Files.createDirectories(pathToArchiveDir);
+
+        Path containerStorageRoot = context1.pathOnHostFromPathInNode("/").getParent();
+        Set<String> containerStorageRootContentsBeforeArchive = FileFinder.from(containerStorageRoot)
+                .maxDepth(1)
+                .stream()
+                .map(FileFinder.FileAttributes::filename)
+                .collect(Collectors.toSet());
+        assertEquals(Set.of("container-archive", "container-1", "container-2"), containerStorageRootContentsBeforeArchive);
+
+
+        // Archive container-1
+        storageMaintainer.archiveNodeStorage(context1);
+
+        clock.advance(Duration.ofSeconds(3));
+        storageMaintainer.archiveNodeStorage(context1);
+
+        // container-1 should be gone from container-storage
+        Set<String> containerStorageRootContentsAfterArchive = FileFinder.from(containerStorageRoot)
+                .maxDepth(1)
+                .stream()
+                .map(FileFinder.FileAttributes::filename)
+                .collect(Collectors.toSet());
+        assertEquals(Set.of("container-archive", "container-2"), containerStorageRootContentsAfterArchive);
+
+        // container archive directory should contain exactly 1 directory - the one we just archived
+        List<FileFinder.FileAttributes> containerArchiveContentsAfterArchive = FileFinder.from(pathToArchiveDir).maxDepth(1).list();
+        assertEquals(1, containerArchiveContentsAfterArchive.size());
+        Path archivedContainerStoragePath = containerArchiveContentsAfterArchive.get(0).path();
+        assertEquals("container-1_20090213233130", archivedContainerStoragePath.getFileName().toString());
+        Set<String> archivedContainerStorageContents = FileFinder.files(archivedContainerStoragePath)
+                .stream()
+                .map(fileAttributes -> archivedContainerStoragePath.relativize(fileAttributes.path()).toString())
+                .collect(Collectors.toSet());
+        assertEquals(Set.of("opt/vespa/logs/vespa/vespa.log", "opt/vespa/logs/vespa/zookeeper.log"), archivedContainerStorageContents);
+    }
 
-        @Test
-        public void testNonExistingDiskUsed() {
-            StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, null, null);
-            DiskSize size = storageMaintainer.getDiskUsed(null, Paths.get("/fake/path"));
-            assertEquals(DiskSize.ZERO, size);
-        }
+    private static NodeAgentContext createNodeAgentContextAndContainerStorage(FileSystem fileSystem, String containerName) throws IOException {
+        NodeAgentContext context = new NodeAgentContextImpl.Builder(containerName + ".domain.tld")
+                .fileSystem(fileSystem).build();
 
-        @After
-        public void after() {
-            terminal.verifyAllCommandsExecuted();
-        }
+        Path containerVespaHomeOnHost = context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome(""));
+        Files.createDirectories(context.pathOnHostFromPathInNode("/etc/something"));
+        Files.createFile(context.pathOnHostFromPathInNode("/etc/something/conf"));
+
+        Files.createDirectories(containerVespaHomeOnHost.resolve("logs/vespa"));
+        Files.createFile(containerVespaHomeOnHost.resolve("logs/vespa/vespa.log"));
+        Files.createFile(containerVespaHomeOnHost.resolve("logs/vespa/zookeeper.log"));
+
+        Files.createDirectories(containerVespaHomeOnHost.resolve("var/db"));
+        Files.createFile(containerVespaHomeOnHost.resolve("var/db/some-file"));
+
+        Path containerRootOnHost = context.pathOnHostFromPathInNode("/");
+        Set<String> actualContents = FileFinder.files(containerRootOnHost)
+                .stream()
+                .map(fileAttributes -> containerRootOnHost.relativize(fileAttributes.path()).toString())
+                .collect(Collectors.toSet());
+        Set<String> expectedContents = Set.of(
+                "etc/something/conf",
+                "opt/vespa/logs/vespa/vespa.log",
+                "opt/vespa/logs/vespa/zookeeper.log",
+                "opt/vespa/var/db/some-file");
+        assertEquals(expectedContents, actualContents);
+        return context;
     }
 
-    public static class ArchiveContainerDataTests {
-        @Test
-        public void archive_container_data_test() throws IOException {
-            // Create some files in containers
-            FileSystem fileSystem = TestFileSystem.create();
-            NodeAgentContext context1 = createNodeAgentContextAndContainerStorage(fileSystem, "container-1");
-            createNodeAgentContextAndContainerStorage(fileSystem, "container-2");
-
-            Path pathToArchiveDir = fileSystem.getPath("/home/docker/container-storage/container-archive");
-            Files.createDirectories(pathToArchiveDir);
-
-            Path containerStorageRoot = context1.pathOnHostFromPathInNode("/").getParent();
-            Set<String> containerStorageRootContentsBeforeArchive = FileFinder.from(containerStorageRoot)
-                    .maxDepth(1)
-                    .stream()
-                    .map(FileFinder.FileAttributes::filename)
-                    .collect(Collectors.toSet());
-            assertEquals(Set.of("container-archive", "container-1", "container-2"), containerStorageRootContentsBeforeArchive);
-
-
-            // Archive container-1
-            ManualClock clock = new ManualClock(Instant.ofEpochSecond(1234567890));
-            StorageMaintainer storageMaintainer = new StorageMaintainer(null, null, pathToArchiveDir, null, clock);
-            storageMaintainer.archiveNodeStorage(context1);
-
-            clock.advance(Duration.ofSeconds(3));
-            storageMaintainer.archiveNodeStorage(context1);
-
-            // container-1 should be gone from container-storage
-            Set<String> containerStorageRootContentsAfterArchive = FileFinder.from(containerStorageRoot)
-                    .maxDepth(1)
-                    .stream()
-                    .map(FileFinder.FileAttributes::filename)
-                    .collect(Collectors.toSet());
-            assertEquals(Set.of("container-archive", "container-2"), containerStorageRootContentsAfterArchive);
-
-            // container archive directory should contain exactly 1 directory - the one we just archived
-            List<FileFinder.FileAttributes> containerArchiveContentsAfterArchive = FileFinder.from(pathToArchiveDir).maxDepth(1).list();
-            assertEquals(1, containerArchiveContentsAfterArchive.size());
-            Path archivedContainerStoragePath = containerArchiveContentsAfterArchive.get(0).path();
-            assertEquals("container-1_20090213233130", archivedContainerStoragePath.getFileName().toString());
-            Set<String> archivedContainerStorageContents = FileFinder.files(archivedContainerStoragePath)
-                    .stream()
-                    .map(fileAttributes -> archivedContainerStoragePath.relativize(fileAttributes.path()).toString())
-                    .collect(Collectors.toSet());
-            assertEquals(Set.of("opt/vespa/logs/vespa/vespa.log", "opt/vespa/logs/vespa/zookeeper.log"), archivedContainerStorageContents);
-        }
-
-        private NodeAgentContext createNodeAgentContextAndContainerStorage(FileSystem fileSystem, String containerName) throws IOException {
-            NodeAgentContext context = new NodeAgentContextImpl.Builder(containerName + ".domain.tld")
-                    .fileSystem(fileSystem).build();
-
-            Path containerVespaHomeOnHost = context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome(""));
-            Files.createDirectories(context.pathOnHostFromPathInNode("/etc/something"));
-            Files.createFile(context.pathOnHostFromPathInNode("/etc/something/conf"));
-
-            Files.createDirectories(containerVespaHomeOnHost.resolve("logs/vespa"));
-            Files.createFile(containerVespaHomeOnHost.resolve("logs/vespa/vespa.log"));
-            Files.createFile(containerVespaHomeOnHost.resolve("logs/vespa/zookeeper.log"));
-
-            Files.createDirectories(containerVespaHomeOnHost.resolve("var/db"));
-            Files.createFile(containerVespaHomeOnHost.resolve("var/db/some-file"));
-
-            Path containerRootOnHost = context.pathOnHostFromPathInNode("/");
-            Set<String> actualContents = FileFinder.files(containerRootOnHost)
-                    .stream()
-                    .map(fileAttributes -> containerRootOnHost.relativize(fileAttributes.path()).toString())
-                    .collect(Collectors.toSet());
-            Set<String> expectedContents = Set.of(
-                    "etc/something/conf",
-                    "opt/vespa/logs/vespa/vespa.log",
-                    "opt/vespa/logs/vespa/zookeeper.log",
-                    "opt/vespa/var/db/some-file");
-            assertEquals(expectedContents, actualContents);
-            return context;
-        }
-    }
+    @Test
+    public void not_run_if_not_enough_used() throws IOException {
+        NodeAgentContext context = new NodeAgentContextImpl.Builder(
+                NodeSpec.Builder.testSpec("h123a.domain.tld").resources(new NodeResources(1, 1, 1, 1)).build())
+                .fileSystem(fileSystem).build();
+        Files.createDirectories(context.pathOnHostFromPathInNode("/"));
+        mockDiskUsage(500L);
 
-    public static class CleanupTests {
+        storageMaintainer.cleanDiskIfFull(context);
+        verifyNoInteractions(diskCleanup);
+    }
 
-        private final TestTerminal terminal = new TestTerminal();
-        private final DiskCleanup diskCleanup = mock(DiskCleanup.class);
-        private final ManualClock clock = new ManualClock();
-        private final StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, null, null, diskCleanup, clock);
-        private final FileSystem fileSystem = TestFileSystem.create();
-        private final NodeAgentContext context = new NodeAgentContextImpl
-                .Builder(NodeSpec.Builder.testSpec("h123a.domain.tld").resources(new NodeResources(1, 1, 1, 1)).build())
+    @Test
+    public void deletes_correct_amount() throws IOException {
+        NodeAgentContext context = new NodeAgentContextImpl.Builder(
+                NodeSpec.Builder.testSpec("h123a.domain.tld").resources(new NodeResources(1, 1, 1, 1)).build())
                 .fileSystem(fileSystem).build();
 
-        @Test
-        public void not_run_if_not_enough_used() throws IOException {
-            Files.createDirectories(context.pathOnHostFromPathInNode("/"));
-            mockDiskUsage(500L);
+        Files.createDirectories(context.pathOnHostFromPathInNode("/"));
+        mockDiskUsage(950_000L);
 
-            storageMaintainer.cleanDiskIfFull(context);
-            verifyNoInteractions(diskCleanup);
-        }
-
-        @Test
-        public void deletes_correct_amount() throws IOException {
-            Files.createDirectories(context.pathOnHostFromPathInNode("/"));
-            mockDiskUsage(950_000L);
+        storageMaintainer.cleanDiskIfFull(context);
+        // Allocated size: 1 GB, usage: 950_000 kiB (972.8 MB). Wanted usage: 70% => 700 MB
+        verify(diskCleanup).cleanup(eq(context), any(), eq(272_800_000L));
+    }
 
-            storageMaintainer.cleanDiskIfFull(context);
-            // Allocated size: 1 GB, usage: 950_000 kiB (972.8 MB). Wanted usage: 70% => 700 MB
-            verify(diskCleanup).cleanup(eq(context), any(), eq(272_800_000L));
-        }
+    @After
+    public void after() {
+        terminal.verifyAllCommandsExecuted();
+    }
 
-        private void mockDiskUsage(long kBytes) {
-            terminal.expectCommand("du -xsk /home/docker/container-storage/h123a 2>&1", 0, kBytes + "\t/path");
-        }
+    private void mockDiskUsage(long kBytes) {
+        terminal.expectCommand("du -xsk /home/docker/container-storage/h123a 2>&1", 0, kBytes + "\t/path");
     }
 }
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 0d596a46d77..9bca907983e 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
@@ -4,19 +4,15 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.sync;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.test.file.TestFileSystem;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.UncheckedIOException;
 import java.nio.file.FileSystem;
-import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
+import java.util.Optional;
 
+import static com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo.Compression.NONE;
+import static com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo.Compression.ZSTD;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
 
 /**
  * @author freva
@@ -28,52 +24,39 @@ public class SyncFileInfoTest {
     private static final String bucket = "logs-region-acdf21";
     private static final ApplicationId application = ApplicationId.from("tenant", "application", "instance");
     private static final HostName hostname = HostName.from("h12352a.env.region-1.vespa.domain.example");
-    private static final Path accessLogPath = fileSystem.getPath("/opt/vespa/logs/qrs/access.json-20210212.zst");
-    private static final Path vespaLogPath = fileSystem.getPath("/opt/vespa/logs/vespa.log-2021-02-12");
+    private static final Path accessLogPath1 = fileSystem.getPath("/opt/vespa/logs/qrs/access.log.20210211");
+    private static final Path accessLogPath2 = fileSystem.getPath("/opt/vespa/logs/qrs/access.log.20210212.zst");
+    private static final Path accessLogPath3 = fileSystem.getPath("/opt/vespa/logs/qrs/access-json.log.20210213.zst");
+    private static final Path accessLogPath4 = fileSystem.getPath("/opt/vespa/logs/qrs/JsonAccessLog.default.20210214.zst");
+    private static final Path connectionLogPath1 = fileSystem.getPath("/opt/vespa/logs/qrs/ConnectionLog.default.20210210");
+    private static final Path connectionLogPath2 = fileSystem.getPath("/opt/vespa/logs/qrs/ConnectionLog.default.20210212.zst");
+    private static final Path vespaLogPath1 = fileSystem.getPath("/opt/vespa/logs/vespa.log");
+    private static final Path vespaLogPath2 = fileSystem.getPath("/opt/vespa/logs/vespa.log-2021-02-12");
 
     @Test
-    public void tenant_access_log() {
-        SyncFileInfo sfi = SyncFileInfo.tenantAccessLog(bucket, application, hostname, accessLogPath);
-        assertEquals(Paths.get("/tenant.application.instance/h12352a/logs/access/access.json-20210212.zst"), sfi.destPath());
-        assertEquals(bucket, sfi.bucketName());
-        assertNotEquals(ZstdCompressingInputStream.class, getInputStreamType(sfi));
-    }
+    public void tenant_log() {
+        assertTenantSyncFileInfo(accessLogPath1, null, null);
+        assertTenantSyncFileInfo(accessLogPath2, "tenant.application.instance/h12352a/logs/access/access.log.20210212.zst", NONE);
+        assertTenantSyncFileInfo(accessLogPath3, "tenant.application.instance/h12352a/logs/access/access-json.log.20210213.zst", NONE);
+        assertTenantSyncFileInfo(accessLogPath4, "tenant.application.instance/h12352a/logs/access/JsonAccessLog.default.20210214.zst", NONE);
 
-    @Test
-    public void tenant_vespa_log() {
-        SyncFileInfo sfi = SyncFileInfo.tenantVespaLog(bucket, application, hostname, vespaLogPath);
-        assertEquals(Paths.get("/tenant.application.instance/h12352a/logs/vespa/vespa.log-2021-02-12.zst"), sfi.destPath());
-        assertEquals(ZstdCompressingInputStream.class, getInputStreamType(sfi));
-    }
+        assertTenantSyncFileInfo(connectionLogPath1, null, null);
+        assertTenantSyncFileInfo(connectionLogPath2, "tenant.application.instance/h12352a/logs/connection/ConnectionLog.default.20210212.zst", NONE);
 
-    @Test
-    public void infra_access_log() {
-        SyncFileInfo sfi = SyncFileInfo.infrastructureAccessLog(bucket, hostname, accessLogPath);
-        assertEquals(Paths.get("/infrastructure/h12352a/logs/access/access.json-20210212.zst"), sfi.destPath());
-        assertNotEquals(ZstdCompressingInputStream.class, getInputStreamType(sfi));
+        assertTenantSyncFileInfo(vespaLogPath1, null, null);
+        assertTenantSyncFileInfo(vespaLogPath2, "tenant.application.instance/h12352a/logs/vespa/vespa.log-2021-02-12.zst", ZSTD);
     }
 
     @Test
     public void infra_vespa_log() {
-        SyncFileInfo sfi = SyncFileInfo.infrastructureVespaLog(bucket, hostname, vespaLogPath);
-        assertEquals(Paths.get("/infrastructure/h12352a/logs/vespa/vespa.log-2021-02-12.zst"), sfi.destPath());
-        assertEquals(ZstdCompressingInputStream.class, getInputStreamType(sfi));
+        SyncFileInfo sfi = SyncFileInfo.infrastructureVespaLog(bucket, hostname, vespaLogPath2);
+        assertEquals("infrastructure/h12352a/logs/vespa/vespa.log-2021-02-12.zst", sfi.destPath().toString());
+        assertEquals(ZSTD, sfi.uploadCompression());
     }
 
-    @BeforeClass
-    public static void setup() throws IOException {
-        Files.createDirectories(vespaLogPath.getParent());
-        Files.createFile(vespaLogPath);
-        Files.createDirectories(accessLogPath.getParent());
-        Files.createFile(accessLogPath);
+    private static void assertTenantSyncFileInfo(Path srcPath, String destPath, SyncFileInfo.Compression compression) {
+        Optional<SyncFileInfo> sfi = SyncFileInfo.tenantLog(bucket, application, hostname, srcPath);
+        assertEquals(destPath, sfi.map(SyncFileInfo::destPath).map(Path::toString).orElse(null));
+        assertEquals(compression, sfi.map(SyncFileInfo::uploadCompression).orElse(null));
     }
-
-    private static Class<? extends InputStream> getInputStreamType(SyncFileInfo syncFileInfo) {
-        try (InputStream inputStream = syncFileInfo.inputStream()) {
-            return inputStream.getClass();
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-    }
-
-}
\ No newline at end of file
+}