Fix metrics

3 files changed, 322 insertions, 135 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 c9e4a8fac7d..390e81affb2 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
@@ -24,6 +24,7 @@ import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -55,71 +56,81 @@ public class StorageMaintainer {
 
     public void writeMetricsConfig(NodeAgentContext context, NodeSpec node) {
         List<SecretAgentCheckConfig> configs = new ArrayList<>();
+        Map<String, Object> tags = generateTags(context, node);
 
         // host-life
         Path hostLifeCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_host_life");
-        SecretAgentCheckConfig hostLifeSchedule = new SecretAgentCheckConfig("host-life", 60, hostLifeCheckPath);
-        configs.add(annotatedCheck(context, node, hostLifeSchedule));
+        configs.add(new SecretAgentCheckConfig("host-life", 60, hostLifeCheckPath).withTags(tags));
 
         // ntp
         Path ntpCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ntp");
-        SecretAgentCheckConfig ntpSchedule = new SecretAgentCheckConfig("ntp", 60, ntpCheckPath);
-        configs.add(annotatedCheck(context, node, ntpSchedule));
+        configs.add(new SecretAgentCheckConfig("ntp", 60, ntpCheckPath).withTags(tags));
 
         // coredumps (except for the done coredumps which is handled by the host)
         Path coredumpCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_coredumps");
-        SecretAgentCheckConfig coredumpSchedule = new SecretAgentCheckConfig("system-coredumps-processing", 300,
-                coredumpCheckPath, "--application", "system-coredumps-processing", "--lastmin",
-                "129600", "--crit", "1", "--coredir", context.pathInNodeUnderVespaHome("var/crash/processing").toString());
-        configs.add(annotatedCheck(context, node, coredumpSchedule));
+        configs.add(new SecretAgentCheckConfig("system-coredumps-processing", 300, coredumpCheckPath,
+                "--application", "system-coredumps-processing",
+                "--lastmin", "129600",
+                "--crit", "1",
+                "--coredir", context.pathInNodeUnderVespaHome("var/crash/processing").toString())
+                .withTags(tags));
 
         // athenz certificate check
         Path athenzCertExpiryCheckPath = context.pathInNodeUnderVespaHome("libexec64/yms/yms_check_athenz_certs");
-        SecretAgentCheckConfig athenzCertExpirySchedule = new SecretAgentCheckConfig("athenz-certificate-expiry", 60,
-                 athenzCertExpiryCheckPath, "--threshold", "20")
-                .withRunAsUser("root");
-        configs.add(annotatedCheck(context, node, athenzCertExpirySchedule));
+        configs.add(new SecretAgentCheckConfig("athenz-certificate-expiry", 60, athenzCertExpiryCheckPath,
+                "--threshold", "20")
+                .withRunAsUser("root")
+                .withTags(tags));
 
         if (context.nodeType() != NodeType.config) {
             // vespa-health
             Path vespaHealthCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_vespa_health");
-            SecretAgentCheckConfig vespaHealthSchedule = new SecretAgentCheckConfig("vespa-health", 60, vespaHealthCheckPath, "all");
-            configs.add(annotatedCheck(context, node, vespaHealthSchedule));
+            configs.add(new SecretAgentCheckConfig("vespa-health", 60, vespaHealthCheckPath, "all").withTags(tags));
 
             // vespa
             Path vespaCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_vespa");
             SecretAgentCheckConfig vespaSchedule = new SecretAgentCheckConfig("vespa", 60, vespaCheckPath, "all");
-            configs.add(annotatedCheck(context, node, vespaSchedule));
+            if (isConfigserverLike(context.nodeType())) {
+                Map<String, Object> tagsWithoutNameSpace = new LinkedHashMap<>(tags);
+                tagsWithoutNameSpace.remove("namespace");
+                vespaSchedule.withTags(tagsWithoutNameSpace);
+            }
+            configs.add(vespaSchedule);
         }
 
-        if (context.nodeType() == NodeType.config) {
+        if (context.nodeType() == NodeType.config || context.nodeType() == NodeType.controller) {
             // configserver
             Path configServerCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ymonsb2");
-            SecretAgentCheckConfig configServerSchedule = new SecretAgentCheckConfig("configserver", 60,
-                    configServerCheckPath, "-zero", "configserver");
-            configs.add(annotatedCheck(context, node, configServerSchedule));
+            configs.add(new SecretAgentCheckConfig(SecretAgentCheckConfig.nodeTypeToRole(context.nodeType()), 60, configServerCheckPath,
+                    "-zero", "configserver")
+                    .withTags(tags));
 
             //zkbackupage
             Path zkbackupCheckPath = context.pathInNodeUnderVespaHome("libexec/yamas2/yms_check_file_age.py");
-            SecretAgentCheckConfig zkbackupSchedule = new SecretAgentCheckConfig("zkbackupage", 300,
-                    zkbackupCheckPath, "-f", context.pathInNodeUnderVespaHome("var/vespa-hosted/zkbackup.stat").toString(),
-                    "-m", "150", "-a", "config-zkbackupage");
-            configs.add(annotatedCheck(context, node, zkbackupSchedule));
+            configs.add(new SecretAgentCheckConfig("zkbackupage", 300, zkbackupCheckPath,
+                    "-f", context.pathInNodeUnderVespaHome("var/vespa-hosted/zkbackup.stat").toString(),
+                    "-m", "150",
+                    "-a", "config-zkbackupage")
+                    .withTags(tags));
         }
 
         if (context.nodeType() == NodeType.proxy) {
             //routing-configage
             Path routingAgeCheckPath = context.pathInNodeUnderVespaHome("libexec/yamas2/yms_check_file_age.py");
-            SecretAgentCheckConfig routingAgeSchedule = new SecretAgentCheckConfig("routing-configage", 60,
-                    routingAgeCheckPath, "-f", context.pathInNodeUnderVespaHome("var/vespa-hosted/routing/nginx.conf.tmp").toString(),
-                    "-m", "1", "-a", "routing-configage", "--ignore_file_not_found");
-            configs.add(annotatedCheck(context, node, routingAgeSchedule));
+            configs.add(new SecretAgentCheckConfig("routing-configage", 60, routingAgeCheckPath,
+                    "-f", context.pathInNodeUnderVespaHome("var/vespa-hosted/routing/nginx.conf.tmp").toString(),
+                    "-m", "1",
+                    "-a", "routing-configage",
+                    "--ignore_file_not_found")
+                    .withTags(tags));
 
             //ssl-check
             Path sslCheckPath = context.pathInNodeUnderVespaHome("libexec/yms/yms_check_ssl_status");
-            SecretAgentCheckConfig sslSchedule = new SecretAgentCheckConfig("ssl-status", 300,
-                    sslCheckPath, "-e", "localhost", "-p", "4443", "-t", "30");
-            configs.add(annotatedCheck(context, node, sslSchedule));
+            configs.add(new SecretAgentCheckConfig("ssl-status", 300, sslCheckPath,
+                    "-e", "localhost",
+                    "-p", "4443",
+                    "-t", "30")
+                    .withTags(tags));
         }
 
         // Write config and restart yamas-agent
@@ -128,26 +139,36 @@ public class StorageMaintainer {
         dockerOperations.executeCommandInContainerAsRoot(context, "service", "yamas-agent", "restart");
     }
 
-    private SecretAgentCheckConfig annotatedCheck(NodeAgentContext context, NodeSpec node, SecretAgentCheckConfig check) {
-        check.withTag("namespace", "Vespa")
-                .withTag("role", SecretAgentCheckConfig.nodeTypeToRole(node.getNodeType()))
-                .withTag("flavor", node.getFlavor())
-                .withTag("canonicalFlavor", node.getCanonicalFlavor())
-                .withTag("state", node.getState().toString())
-                .withTag("zone", String.format("%s.%s", context.zoneId().environment().value(), context.zoneId().regionName().value()));
-        node.getParentHostname().ifPresent(parent -> check.withTag("parentHostname", parent));
-        node.getOwner().ifPresent(owner -> check
-                .withTag("tenantName", owner.getTenant())
-                .withTag("app", owner.getApplication() + "." + owner.getInstance())
-                .withTag("applicationName", owner.getApplication())
-                .withTag("instanceName", owner.getInstance())
-                .withTag("applicationId", owner.getTenant() + "." + owner.getApplication() + "." + owner.getInstance()));
-        node.getMembership().ifPresent(membership -> check
-                .withTag("clustertype", membership.getClusterType())
-                .withTag("clusterid", membership.getClusterId()));
-        node.getVespaVersion().ifPresent(version -> check.withTag("vespaVersion", version));
-
-        return check;
+    private Map<String, Object> generateTags(NodeAgentContext context, NodeSpec node) {
+        Map<String, String> tags = new LinkedHashMap<>();
+        tags.put("namespace", "Vespa");
+        tags.put("role", SecretAgentCheckConfig.nodeTypeToRole(node.getNodeType()));
+        tags.put("zone", String.format("%s.%s", context.zoneId().environment().value(), context.zoneId().regionName().value()));
+        node.getVespaVersion().ifPresent(version -> tags.put("vespaVersion", version));
+
+        if (! isConfigserverLike(context.nodeType())) {
+            tags.put("flavor", node.getFlavor());
+            tags.put("canonicalFlavor", node.getCanonicalFlavor());
+            tags.put("state", node.getState().toString());
+            node.getParentHostname().ifPresent(parent -> tags.put("parentHostname", parent));
+            node.getOwner().ifPresent(owner -> {
+                tags.put("tenantName", owner.getTenant());
+                tags.put("app", owner.getApplication() + "." + owner.getInstance());
+                tags.put("applicationName", owner.getApplication());
+                tags.put("instanceName", owner.getInstance());
+                tags.put("applicationId", owner.getTenant() + "." + owner.getApplication() + "." + owner.getInstance());
+            });
+            node.getMembership().ifPresent(membership -> {
+                tags.put("clustertype", membership.getClusterType());
+                tags.put("clusterid", membership.getClusterId());
+            });
+        }
+
+        return Collections.unmodifiableMap(tags);
+    }
+
+    private boolean isConfigserverLike(NodeType nodeType) {
+        return nodeType == NodeType.config || nodeType == NodeType.controller;
     }
 
     public Optional<Long> getDiskUsageFor(NodeAgentContext context) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java
index f9e5f7c659d..cdf67871a1a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SecretAgentCheckConfig.java
@@ -40,6 +40,12 @@ public class SecretAgentCheckConfig {
         return this;
     }
 
+    public SecretAgentCheckConfig withTags(Map<String, Object> tags) {
+        this.tags.clear();
+        this.tags.putAll(tags);
+        return this;
+    }
+
     public void setTags(Map<String, Object> tags) {
         this.tags.clear();
         this.tags.putAll(tags);
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 42b161b8e2d..d537cb40214 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
@@ -2,14 +2,24 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance;
 
 import com.google.common.collect.ImmutableSet;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.NodeType;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.vespa.hosted.node.admin.component.ZoneId;
+import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 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;
+import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
+import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
 import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
 
 import java.io.IOException;
 import java.nio.file.FileSystem;
@@ -20,7 +30,9 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
+import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.uncheck;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -29,99 +41,247 @@ import static org.mockito.Mockito.mock;
 /**
  * @author dybis
  */
+@RunWith(Enclosed.class)
 public class StorageMaintainerTest {
-    private final DockerOperations docker = mock(DockerOperations.class);
+    private static final DockerOperations docker = mock(DockerOperations.class);
 
-    @Rule
-    public TemporaryFolder folder = new TemporaryFolder();
+    public static class SecretAgentCheckTests {
+        private final StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null);
 
-    @Test
-    public void testDiskUsed() throws IOException, InterruptedException {
-        StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null);
-        int writeSize = 10000;
-        Files.write(folder.newFile().toPath(), new byte[writeSize]);
+        @Test
+        public void tenant() {
+            Path path = executeAs(NodeType.tenant);
 
-        long usedBytes = storageMaintainer.getDiskUsedInBytes(folder.getRoot().toPath());
-        if (usedBytes * 4 < writeSize || usedBytes > writeSize * 4)
-            fail("Used bytes is " + usedBytes + ", but wrote " + writeSize + " bytes, not even close.");
-    }
+            assertChecks(path, "athenz-certificate-expiry", "host-life", "ntp",
+                    "system-coredumps-processing", "vespa", "vespa-health");
+
+            // All dimensions for vespa metrics should be set by metricsproxy
+            assertCheckEnds(path.resolve("vespa.yaml"),
+                    "  args:\n" +
+                    "    - all\n");
+
+            // For non vespa metrics, we need to set all the dimensions ourselves
+            assertCheckEnds(path.resolve("ntp.yaml"),
+                    "tags:\n" +
+                    "    namespace: Vespa\n" +
+                    "    role: tenants\n" +
+                    "    zone: prod.us-north-1\n" +
+                    "    vespaVersion: 6.305.12\n" +
+                    "    flavor: d-2-8-50\n" +
+                    "    canonicalFlavor: d-2-8-50\n" +
+                    "    state: active\n" +
+                    "    parentHostname: host123.test.domain.tld\n" +
+                    "    tenantName: tenant\n" +
+                    "    app: application.instance\n" +
+                    "    applicationName: application\n" +
+                    "    instanceName: instance\n" +
+                    "    applicationId: tenant.application.instance\n" +
+                    "    clustertype: clusterType\n" +
+                    "    clusterid: clusterId\n");
+        }
+
+        @Test
+        public void proxy() {
+            Path path = executeAs(NodeType.proxy);
+
+            assertChecks(path, "athenz-certificate-expiry", "host-life", "ntp", "routing-configage",
+                    "ssl-status", "system-coredumps-processing", "vespa", "vespa-health");
+
+            // All dimensions for vespa metrics should be set by the source
+            assertCheckEnds(path.resolve("vespa.yaml"),
+                    "  args:\n" +
+                    "    - all\n");
+
+            // For non vespa metrics, we need to set all the dimensions ourselves
+            assertCheckEnds(path.resolve("ntp.yaml"),
+                    "tags:\n" +
+                    "    namespace: Vespa\n" +
+                    "    role: routing\n" +
+                    "    zone: prod.us-north-1\n" +
+                    "    vespaVersion: 6.305.12\n" +
+                    "    flavor: d-2-8-50\n" +
+                    "    canonicalFlavor: d-2-8-50\n" +
+                    "    state: active\n" +
+                    "    parentHostname: host123.test.domain.tld\n" +
+                    "    tenantName: tenant\n" +
+                    "    app: application.instance\n" +
+                    "    applicationName: application\n" +
+                    "    instanceName: instance\n" +
+                    "    applicationId: tenant.application.instance\n" +
+                    "    clustertype: clusterType\n" +
+                    "    clusterid: clusterId\n");
+        }
+
+        @Test
+        public void configserver() {
+            Path path = executeAs(NodeType.config);
+
+            assertChecks(path, "athenz-certificate-expiry", "configserver", "host-life", "ntp",
+                    "system-coredumps-processing", "zkbackupage");
+
+            assertCheckEnds(path.resolve("configserver.yaml"),
+                    "  tags:\n" +
+                    "    namespace: Vespa\n" +
+                    "    role: configserver\n" +
+                    "    zone: prod.us-north-1\n" +
+                    "    vespaVersion: 6.305.12\n");
+        }
+
+        @Test
+        public void controller() {
+            Path path = executeAs(NodeType.controller);
+
+            assertChecks(path, "athenz-certificate-expiry", "controller", "host-life", "ntp",
+                    "system-coredumps-processing", "vespa", "vespa-health", "zkbackupage");
+
+
+            // Do not set namespace for vespa metrics. WHY?
+            assertCheckEnds(path.resolve("vespa.yaml"),
+                    "  tags:\n" +
+                    "    role: controller\n" +
+                    "    zone: prod.us-north-1\n" +
+                    "    vespaVersion: 6.305.12\n");
+
+            assertCheckEnds(path.resolve("controller.yaml"),
+                    "  tags:\n" +
+                    "    namespace: Vespa\n" +
+                    "    role: controller\n" +
+                    "    zone: prod.us-north-1\n" +
+                    "    vespaVersion: 6.305.12\n");
+        }
+
+        private Path executeAs(NodeType nodeType) {
+            NodeAgentContext context = new NodeAgentContextImpl.Builder("host123-5.test.domain.tld")
+                    .nodeType(nodeType)
+                    .fileSystem(TestFileSystem.create())
+                    .zoneId(new ZoneId(SystemName.dev, Environment.prod, RegionName.from("us-north-1"))).build();
+            NodeSpec nodeSpec = new NodeSpec.Builder()
+                    .hostname(context.hostname().value())
+                    .nodeType(nodeType)
+                    .state(Node.State.active)
+                    .parentHostname("host123.test.domain.tld")
+                    .owner(new NodeSpec.Owner("tenant", "application", "instance"))
+                    .membership(new NodeSpec.Membership("clusterType", "clusterId", null, 0, false))
+                    .vespaVersion("6.305.12")
+                    .flavor("d-2-8-50")
+                    .canonicalFlavor("d-2-8-50")
+                    .build();
+            Path path = context.pathOnHostFromPathInNode("/etc/yamas-agent");
+            uncheck(() -> Files.createDirectories(path));
+            storageMaintainer.writeMetricsConfig(context, nodeSpec);
+            return path;
+        }
 
-    @Test
-    public void testNonExistingDiskUsed() throws IOException, InterruptedException {
-        StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null);
-        long usedBytes = storageMaintainer.getDiskUsedInBytes(folder.getRoot().toPath().resolve("doesn't exist"));
-        assertEquals(0L, usedBytes);
+        private void assertCheckEnds(Path checkPath, String contentsEnd) {
+            String contents = new UnixPath(checkPath).readUtf8File();
+            assertTrue(contents, contents.endsWith(contentsEnd));
+        }
+
+        private void assertChecks(Path checksPath, String... checkNames) {
+            List<String> expectedChecks = Stream.of(checkNames).sorted().collect(Collectors.toList());
+            List<String> actualChecks = FileFinder.files(checksPath).stream()
+                    .map(FileFinder.FileAttributes::filename)
+                    .map(filename -> filename.replaceAll("\\.yaml$", ""))
+                    .sorted()
+                    .collect(Collectors.toList());
+            assertEquals(expectedChecks, actualChecks);
+        }
     }
 
-    @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-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(ImmutableSet.of("container-archive", "container-1", "container-2"), containerStorageRootContentsBeforeArchive);
-
-
-        // Archive container-1
-        StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, pathToArchiveDir);
-        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(ImmutableSet.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();
-        assertTrue(archivedContainerStoragePath.getFileName().toString().matches("container-1_[0-9]{14}"));
-        Set<String> archivedContainerStorageContents = FileFinder.files(archivedContainerStoragePath)
-                .stream()
-                .map(fileAttributes -> archivedContainerStoragePath.relativize(fileAttributes.path()).toString())
-                .collect(Collectors.toSet());
-        assertEquals(ImmutableSet.of("opt/vespa/logs/vespa/vespa.log", "opt/vespa/logs/vespa/zookeeper.log"), archivedContainerStorageContents);
+    public static class DiskUsageTests {
+        @Rule
+        public TemporaryFolder folder = new TemporaryFolder();
+
+        @Test
+        public void testDiskUsed() throws IOException, InterruptedException {
+            StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null);
+            int writeSize = 10000;
+            Files.write(folder.newFile().toPath(), new byte[writeSize]);
+
+            long usedBytes = storageMaintainer.getDiskUsedInBytes(folder.getRoot().toPath());
+            if (usedBytes * 4 < writeSize || usedBytes > writeSize * 4)
+                fail("Used bytes is " + usedBytes + ", but wrote " + writeSize + " bytes, not even close.");
+        }
+
+        @Test
+        public void testNonExistingDiskUsed() throws IOException, InterruptedException {
+            StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, null);
+            long usedBytes = storageMaintainer.getDiskUsedInBytes(folder.getRoot().toPath().resolve("doesn't exist"));
+            assertEquals(0L, usedBytes);
+        }
     }
 
-    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 = new HashSet<>(Arrays.asList(
-                "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-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(ImmutableSet.of("container-archive", "container-1", "container-2"), containerStorageRootContentsBeforeArchive);
+
+
+            // Archive container-1
+            StorageMaintainer storageMaintainer = new StorageMaintainer(docker, null, pathToArchiveDir);
+            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(ImmutableSet.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();
+            assertTrue(archivedContainerStoragePath.getFileName().toString().matches("container-1_[0-9]{14}"));
+            Set<String> archivedContainerStorageContents = FileFinder.files(archivedContainerStoragePath)
+                    .stream()
+                    .map(fileAttributes -> archivedContainerStoragePath.relativize(fileAttributes.path()).toString())
+                    .collect(Collectors.toSet());
+            assertEquals(ImmutableSet.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 = new HashSet<>(Arrays.asList(
+                    "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;
+        }
     }
 }