Add noManagedContainersRunning() that hides impl in DockerImpl

5 files changed, 36 insertions, 81 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerLite.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerLite.java
deleted file mode 100644
index 51d815a412f..00000000000
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/ContainerLite.java
+++ /dev/null
@@ -1,61 +0,0 @@
-// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.dockerapi;
-
-import java.util.Objects;
-
-/**
- * Information about a container as a result of listing containers (aka "docker ps").
- *
- * @author hakonhall
- */
-public class ContainerLite {
-    private final String id;
-    private final String imageId;
-    // state is one of: "running"
-    private final String state;
-
-    public ContainerLite(String id, String imageId, String state) {
-        this.id = id;
-        this.imageId = imageId;
-        this.state = state;
-    }
-
-    /** Of format: "94a66101b8dfbf485f4f77a448b079684ea704927aa39e31d824de708cfa3373" */
-    public String id() {
-        return id;
-    }
-
-    /** Of format: "sha256:7f3abbbbb17d135840a1f185ac291c87f7b90651e65b6021e820abaf397dd282" */
-    public String imageId() {
-        return imageId;
-    }
-
-    /** Whether the container is running. */
-    public boolean isRunning() {
-        return "running".equals(state);
-    }
-
-    @Override
-    public String toString() {
-        return "ContainerLite{" +
-                "id='" + id + '\'' +
-                ", imageId='" + imageId + '\'' +
-                ", state='" + state + '\'' +
-                '}';
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        ContainerLite that = (ContainerLite) o;
-        return Objects.equals(id, that.id) &&
-                Objects.equals(imageId, that.imageId) &&
-                Objects.equals(state, that.state);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(id, imageId, state);
-    }
-}
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
index 1729c4843ef..4e7ef5a1ff6 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
@@ -81,12 +81,8 @@ public interface Docker {
      */
     boolean pullImageAsyncIfNeeded(DockerImage image);
 
-    /** List all containers, including those not running. */
-    List<ContainerLite> listAllContainers();
+    boolean noManagedContainersRunning(String manager);
 
-    /**
-     * Deletes the local images that are currently not in use by any container and not recently used.
-     */
     boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete);
 
     /**
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java
index 242332ebd54..4eeb00c7685 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollector.java
@@ -73,7 +73,7 @@ class DockerImageGarbageCollector {
      */
     boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete) {
         List<Image> images = docker.listAllImages();
-        List<ContainerLite> containers = docker.listAllContainers();
+        List<Container> containers = docker.listAllContainers();
 
         Map<String, Image> imageByImageId = images.stream().collect(Collectors.toMap(Image::getId, Function.identity()));
 
@@ -138,14 +138,14 @@ class DockerImageGarbageCollector {
                 .count() > 0;
     }
 
-    private Set<String> getRecentlyUsedImageIds(List<Image> images, List<ContainerLite> containers, Duration minImageAgeToDelete) {
+    private Set<String> getRecentlyUsedImageIds(List<Image> images, List<Container> containers, Duration minImageAgeToDelete) {
         final Instant now = clock.instant();
 
         // Add any already downloaded image to the list once
         images.forEach(image -> lastTimeUsedByImageId.putIfAbsent(image.getId(), now));
 
         // Update last used time for all current containers
-        containers.forEach(container -> lastTimeUsedByImageId.put(container.imageId(), now));
+        containers.forEach(container -> lastTimeUsedByImageId.put(container.getImageId(), now));
 
         // Return list of images that have been used within minImageAgeToDelete
         return lastTimeUsedByImageId.entrySet().stream()
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
index 683c8a98788..8cdb0bee7c2 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
@@ -42,7 +42,6 @@ import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 public class DockerImpl implements Docker {
@@ -298,14 +297,15 @@ public class DockerImpl implements Docker {
     }
 
     @Override
-    public List<ContainerLite> listAllContainers() {
+    public boolean noManagedContainersRunning(String manager) {
+        return listAllContainers().stream()
+                .filter(container -> isManagedBy(container, manager))
+                .noneMatch(container -> "running".equalsIgnoreCase(container.getState()));
+    }
+
+    List<com.github.dockerjava.api.model.Container> listAllContainers() {
         try {
-            return dockerClient.listContainersCmd().withShowAll(true).exec().stream()
-                    .map(c -> new ContainerLite(
-                            c.getId(), // Example: "94a66101b8dfbf485f4f77a448b079684ea704927aa39e31d824de708cfa3373"
-                            c.getImageId(), // Example: "sha256:7f3abbbbb17d135840a1f185ac291c87f7b90651e65b6021e820abaf397dd282"
-                            c.getState())) // Example: "running"
-                    .collect(Collectors.toList());
+            return dockerClient.listContainersCmd().withShowAll(true).exec();
         } catch (RuntimeException e) {
             numberOfDockerDaemonFails.add();
             throw new DockerException("Failed to list all containers", e);
diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java
index 520f8a74d58..412b8301e4c 100644
--- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java
+++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImageGarbageCollectionTest.java
@@ -50,7 +50,7 @@ public class DockerImageGarbageCollectionTest {
     @Test
     public void singleImageWithContainerIsUsed() {
         gcTester.withExistingImages(ImageBuilder.forId("image-1"))
-                .andExistingContainers(new ContainerLite("container-1", "image-1", "running"))
+                .andExistingContainers(ContainerBuilder.forId("container-1").withImageId("image-1"))
                 .expectDeletedImages();
     }
 
@@ -77,7 +77,7 @@ public class DockerImageGarbageCollectionTest {
                         ImageBuilder.forId("parent-image"),
                         ImageBuilder.forId("image-1").withParentId("parent-image").withTags("latest"),
                         ImageBuilder.forId("image-2").withParentId("parent-image").withTags("1.24"))
-                .andExistingContainers(new ContainerLite("vespa-node-1", "image-1", "running"))
+                .andExistingContainers(ContainerBuilder.forId("vespa-node-1").withImageId("image-1"))
                 .expectDeletedImages("1.24"); // Deleting the only tag will delete the image
     }
 
@@ -173,8 +173,10 @@ public class DockerImageGarbageCollectionTest {
             return this;
         }
 
-        private ImageGcTester andExistingContainers(ContainerLite... containers) {
-            when(docker.listAllContainers()).thenReturn(List.of(containers));
+        private ImageGcTester andExistingContainers(ContainerBuilder... containers) {
+            when(docker.listAllContainers()).thenReturn(Arrays.stream(containers)
+                    .map(ContainerBuilder::toContainer)
+                    .collect(Collectors.toList()));
             return this;
         }
 
@@ -255,4 +257,22 @@ public class DockerImageGarbageCollectionTest {
         private ImageBuilder withTags(String... tags) { this.repoTags = tags; return this; }
         private Image toImage() { return createFrom(Image.class, this); }
     }
+
+    // Workaround for Container class that can't be instantiated directly in Java (instantiate via Jackson instead).
+    private static class ContainerBuilder {
+        // Json property names must match exactly the property names in the Container class.
+        @JsonProperty("Id")
+        private final String id;
+
+        @JsonProperty("ImageID")
+        private String imageId;
+
+        private ContainerBuilder(String id) { this.id = id; }
+        private static ContainerBuilder forId(final String id) { return new ContainerBuilder(id); }
+        private ContainerBuilder withImageId(String imageId) { this.imageId = imageId; return this; }
+
+        private com.github.dockerjava.api.model.Container toContainer() {
+            return createFrom(com.github.dockerjava.api.model.Container.class, this);
+        }
+    }
 }