diff options
Diffstat (limited to 'docker-api/src')
4 files changed, 57 insertions, 71 deletions
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 52de3bf91eb..171426da60f 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 @@ -6,10 +6,8 @@ import com.github.dockerjava.api.command.ExecCreateCmdResponse; import com.github.dockerjava.api.command.ExecStartCmd; import com.github.dockerjava.api.command.InspectContainerResponse; import com.github.dockerjava.api.command.InspectExecResponse; -import com.github.dockerjava.api.command.InspectImageResponse; import com.github.dockerjava.api.exception.DockerClientException; import com.github.dockerjava.api.exception.DockerException; -import com.github.dockerjava.api.exception.NotFoundException; import com.github.dockerjava.api.model.Image; import com.github.dockerjava.api.model.Network; import com.github.dockerjava.api.model.Statistics; @@ -48,15 +46,14 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import static com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator.NetworkAddressInterface; public class DockerImpl implements Docker { private static final Logger logger = Logger.getLogger(DockerImpl.class.getName()); - private static final String LABEL_NAME_MANAGEDBY = "com.yahoo.vespa.managedby"; - private static final String LABEL_VALUE_MANAGEDBY = "node-admin"; + public static final String LABEL_NAME_MANAGEDBY = "com.yahoo.vespa.managedby"; + public static final String LABEL_VALUE_MANAGEDBY = "node-admin"; private final int SECONDS_TO_WAIT_BEFORE_KILLING; private static final String FRAMEWORK_CONTAINER_PREFIX = "/"; @@ -192,11 +189,10 @@ public class DockerImpl implements Docker { return inspectImage(dockerImage).isPresent(); } - private Optional<InspectImageResponse> inspectImage(DockerImage dockerImage) { + private Optional<Image> inspectImage(DockerImage dockerImage) { try { - return Optional.of(dockerClient.inspectImageCmd(dockerImage.asString()).exec()); - } catch (NotFoundException e) { - return Optional.empty(); + return dockerClient.listImagesCmd().withShowAll(true) + .withImageNameFilter(dockerImage.asString()).exec().stream().findFirst(); } catch (DockerException e) { numberOfDockerDaemonFails.add(); throw new RuntimeException("Failed to list image name: '" + dockerImage + "'", e); @@ -205,8 +201,7 @@ public class DockerImpl implements Docker { @Override public CreateContainerCommand createContainerCommand(DockerImage image, ContainerName name, String hostName) { - return new CreateContainerCommandImpl(dockerClient, image, name, hostName) - .withLabel(LABEL_NAME_MANAGEDBY, LABEL_VALUE_MANAGEDBY); + return new CreateContainerCommandImpl(dockerClient, image, name, hostName); } @Override @@ -275,7 +270,7 @@ public class DockerImpl implements Docker { @Override public void startContainer(ContainerName containerName) { - Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName, true); + Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName); if (dockerContainer.isPresent()) { try { dockerClient.startContainerCmd(dockerContainer.get().getId()).exec(); @@ -289,7 +284,7 @@ public class DockerImpl implements Docker { @Override public void stopContainer(final ContainerName containerName) { - Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName, true); + Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName); if (dockerContainer.isPresent()) { try { dockerClient.stopContainerCmd(dockerContainer.get().getId()).withTimeout(SECONDS_TO_WAIT_BEFORE_KILLING).exec(); @@ -302,7 +297,7 @@ public class DockerImpl implements Docker { @Override public void deleteContainer(ContainerName containerName) { - Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName, true); + Optional<com.github.dockerjava.api.model.Container> dockerContainer = getContainerFromName(containerName); if (dockerContainer.isPresent()) { dockerImageGC.ifPresent(imageGC -> imageGC.updateLastUsedTimeFor(dockerContainer.get().getImageId())); @@ -316,35 +311,38 @@ public class DockerImpl implements Docker { } } - @Override - public List<Container> getAllManagedContainers() { + List<Container> getAllContainers(boolean managedOnly) { try { return dockerClient.listContainersCmd().withShowAll(true).exec().stream() - .filter(this::isManaged) - .flatMap(this::asContainer) + .filter(container -> !managedOnly || isManaged(container)) + .map(this::asContainer) .collect(Collectors.toList()); } catch (DockerException e) { numberOfDockerDaemonFails.add(); - throw new RuntimeException("Could not retrieve all container", e); + throw new RuntimeException("Could not retrieve all containers", e); } } @Override + public List<Container> getAllManagedContainers() { + return getAllContainers(true); + } + + @Override public Optional<Container> getContainer(String hostname) { - // TODO Don't rely on getAllManagedContainers - return getAllManagedContainers().stream() + return getAllContainers(false).stream() .filter(c -> Objects.equals(hostname, c.hostname)) .findFirst(); } - private Stream<Container> asContainer(com.github.dockerjava.api.model.Container dockerClientContainer) { + private Container asContainer(com.github.dockerjava.api.model.Container dockerClientContainer) { try { final InspectContainerResponse response = dockerClient.inspectContainerCmd(dockerClientContainer.getId()).exec(); - return Stream.of(new Container( + return new Container( response.getConfig().getHostName(), new DockerImage(dockerClientContainer.getImage()), new ContainerName(decode(response.getName())), - response.getState().getRunning())); + response.getState().getRunning()); } catch (DockerException e) { numberOfDockerDaemonFails.add(); //TODO: do proper exception handling @@ -353,11 +351,9 @@ public class DockerImpl implements Docker { } - private Optional<com.github.dockerjava.api.model.Container> getContainerFromName( - final ContainerName containerName, final boolean alsoGetStoppedContainers) { + private Optional<com.github.dockerjava.api.model.Container> getContainerFromName(final ContainerName containerName) { try { - return dockerClient.listContainersCmd().withShowAll(alsoGetStoppedContainers).exec().stream() - .filter(this::isManaged) + return dockerClient.listContainersCmd().withShowAll(true).exec().stream() .filter(container -> matchName(container, containerName.asString())) .findFirst(); } catch (DockerException e) { @@ -396,8 +392,8 @@ public class DockerImpl implements Docker { public void buildImage(File dockerfile, DockerImage image) { try { dockerClient.buildImageCmd(dockerfile).withTag(image.asString()) - .exec(new BuildImageResultCallback()).awaitCompletion(); - } catch (DockerException | InterruptedException e) { + .exec(new BuildImageResultCallback()).awaitImageId(); + } catch (DockerException e) { numberOfDockerDaemonFails.add(); throw new RuntimeException("Failed to build image " + image.asString(), e); } @@ -428,9 +424,9 @@ public class DockerImpl implements Docker { @Override public void onComplete() { - Optional<InspectImageResponse> inspectImage = inspectImage(dockerImage); - if (inspectImage.isPresent()) { // Download successful, update image GC with the newly downloaded image - dockerImageGC.ifPresent(imageGC -> imageGC.updateLastUsedTimeFor(inspectImage.get().getId())); + Optional<Image> image = inspectImage(dockerImage); + if (image.isPresent()) { // Download successful, update image GC with the newly downloaded image + dockerImageGC.ifPresent(imageGC -> imageGC.updateLastUsedTimeFor(image.get().getId())); removeScheduledPoll(dockerImage).complete(dockerImage); } else { removeScheduledPoll(dockerImage).completeExceptionally( diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java index 30766392e01..ecee17f1b3b 100644 --- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java +++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerTestUtils.java @@ -10,6 +10,17 @@ import java.io.IOException; import java.util.concurrent.ExecutionException; /** + * Helper class for testing full integration with docker daemon, requires running daemon. To run these tests: + * + * MAC: + * 1. Install Docker Toolbox, and start it (Docker Quickstart Terminal) (you can close terminal window afterwards) + * 2. For network test, we need to make docker containers visible for Mac: sudo route add 172.18.0.0/16 192.168.99.100 + * + * GENERAL TIPS: + * For cleaning up your local docker machine (DON'T DO THIS ON PROD) + * docker stop $(docker ps -a -q) + * docker rm $(docker ps -a -q) + * * @author freva */ public class DockerTestUtils { @@ -28,7 +39,7 @@ public class DockerTestUtils { public static boolean dockerDaemonIsPresent() { if (docker != null) return true; if (operatingSystem == OS.Unsupported) { - System.out.println("This test does not support " + System.getProperty("os.name") + " yet, ignoring test."); + System.err.println("This test does not support " + System.getProperty("os.name") + " yet, ignoring test."); return false; } 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 fd2ab6f8b5f..72fe753c82b 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 @@ -45,15 +45,6 @@ public class DockerImageGarbageCollectionTest { } @Test - public void onlyLeafImageIsUnused() throws Exception { - new ImageGcTester(0) - .withExistingImages( - ImageBuilder.forId("parent-image"), - ImageBuilder.forId("leaf-image").withParentId("parent-image")) - .expectUnusedImages("leaf-image", "parent-image"); - } - - @Test public void multipleUnusedImagesAreIdentified() throws Exception { new ImageGcTester(0) .withExistingImages( diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java index 473de689a13..df911c334d7 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerTest.java @@ -9,7 +9,7 @@ import org.junit.Test; import java.io.IOException; import java.net.InetAddress; import java.net.URL; -import java.util.List; +import java.util.Optional; import java.util.concurrent.ExecutionException; import static org.hamcrest.CoreMatchers.is; @@ -19,18 +19,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; /** - * Class for testing full integration with docker daemon, requires running daemon. To run these tests: - * - * MAC: - * 1. Install Docker Toolbox, and start it (Docker Quickstart Terminal) (you can close terminal window afterwards) - * 2. For network test, we need to make docker containers visible for Mac: sudo route add 172.18.0.0/16 192.168.99.100 - * 2. Run tests from IDE/mvn. - * - * - * TIPS: - * For cleaning up your local docker machine (DON'T DO THIS ON PROD) - * docker stop $(docker ps -a -q) - * docker rm $(docker ps -a -q) + * Requires docker daemon, see {@link com.yahoo.vespa.hosted.dockerapi.DockerTestUtils} for more details. * * @author freva * @author dybdahl @@ -95,24 +84,23 @@ public class DockerTest { @Test public void testContainerCycle() throws IOException, InterruptedException, ExecutionException { - ContainerName containerName = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + "foo"); - docker.createContainerCommand(dockerImage, containerName, "hostName1").create(); - List<Container> managedContainers = docker.getAllManagedContainers(); - assertThat(managedContainers.size(), is(1)); - assertThat(managedContainers.get(0).name, is(containerName)); - assertThat(managedContainers.get(0).isRunning, is(false)); + final ContainerName containerName = new ContainerName(CONTAINER_NAME_DOCKER_TEST_PREFIX + "foo"); + final String containerHostname = "hostName1"; + + docker.createContainerCommand(dockerImage, containerName, containerHostname).create(); + Optional<Container> container = docker.getContainer(containerHostname); + assertTrue(container.isPresent()); + assertFalse(container.get().isRunning); docker.startContainer(containerName); - managedContainers = docker.getAllManagedContainers(); - assertThat(managedContainers.size(), is(1)); - assertThat(managedContainers.get(0).name, is(containerName)); - assertThat(managedContainers.get(0).isRunning, is(true)); + container = docker.getContainer(containerHostname); + assertTrue(container.isPresent()); + assertTrue(container.get().isRunning); docker.stopContainer(containerName); - managedContainers = docker.getAllManagedContainers(); - assertThat(managedContainers.size(), is(1)); - assertThat(managedContainers.get(0).name, is(containerName)); - assertThat(managedContainers.get(0).isRunning, is(false)); + container = docker.getContainer(containerHostname); + assertTrue(container.isPresent()); + assertFalse(container.get().isRunning); docker.deleteContainer(containerName); assertThat(docker.getAllManagedContainers().isEmpty(), is(true)); @@ -160,7 +148,7 @@ public class DockerTest { } // Clean up any non deleted containers from previous tests - docker.getAllManagedContainers().forEach(container -> { + docker.getAllContainers(false).forEach(container -> { if (container.name.asString().startsWith(CONTAINER_NAME_DOCKER_TEST_PREFIX)) { if (container.isRunning) docker.stopContainer(container.name); docker.deleteContainer(container.name); |