diff options
author | Valerij Fredriksen <valerijf@oath.com> | 2018-09-20 13:59:48 +0200 |
---|---|---|
committer | Valerij Fredriksen <valerijf@oath.com> | 2018-09-20 13:59:48 +0200 |
commit | 986839d97de50637fa3551d24ddf802bb5106881 (patch) | |
tree | eda30b163c8563c13539d9b6b89d69332949e3b7 /docker-api | |
parent | 37a5f65f3ee16fab0e3b743eef385a4e2ddb34e1 (diff) |
Simplify DockerImpl
Diffstat (limited to 'docker-api')
3 files changed, 30 insertions, 71 deletions
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 966b5da9def..8bde491d83b 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 @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.dockerapi; import java.net.InetAddress; +import java.time.Duration; import java.util.List; import java.util.Optional; @@ -11,7 +12,8 @@ import java.util.Optional; */ public interface Docker { /** - * Must be called before any other method. May be called more than once. + * Should only be called by non-host-admin. May be called more than once. + * TODO: Remove when migration to host-admin is done */ void start(); @@ -86,7 +88,7 @@ public interface Docker { /** * Deletes the local images that are currently not in use by any container and not recently used. */ - void deleteUnusedDockerImages(); + boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete); /** * Execute a command in docker container as $VESPA_USER. Will block until the command is finished. 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 00753f59461..69ab697ea27 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 @@ -53,58 +53,37 @@ import static com.yahoo.vespa.hosted.dockerapi.DockerNetworkCreator.NetworkAddre public class DockerImpl implements Docker { private static final Logger logger = Logger.getLogger(DockerImpl.class.getName()); - private static final String DOCKER_CUSTOM_MACVLAN_NETWORK_NAME = "vespa-macvlan"; static final String LABEL_NAME_MANAGEDBY = "com.yahoo.vespa.managedby"; + private static final String DOCKER_CUSTOM_MACVLAN_NETWORK_NAME = "vespa-macvlan"; private static final String FRAMEWORK_CONTAINER_PREFIX = "/"; - - private final DockerConfig config; - private final Optional<DockerImageGarbageCollector> dockerImageGC; - private final int secondsToWaitBeforeKilling; - private CounterWrapper numberOfDockerDaemonFails; - private boolean started = false; + private static final int SECONDS_TO_WAIT_BEFORE_KILLING = 10; private final Object monitor = new Object(); private final Set<DockerImage> scheduledPulls = new HashSet<>(); - private DockerClient dockerClient; + private final DockerClient dockerClient; + private final DockerImageGarbageCollector dockerImageGC; + private final CounterWrapper numberOfDockerDaemonFails; @Inject public DockerImpl(DockerConfig config, MetricReceiverWrapper metricReceiverWrapper) { - this.config = config; - - secondsToWaitBeforeKilling = Optional.ofNullable(config) - .map(DockerConfig::secondsToWaitBeforeKillingContainer) - .orElse(10); - - dockerImageGC = Optional.ofNullable(config) - .map(DockerConfig::imageGCMinTimeToLiveMinutes) - .map(Duration::ofMinutes) - .map(DockerImageGarbageCollector::new); - - Optional.ofNullable(metricReceiverWrapper).ifPresent(this::setMetrics); + this(createDockerClient(config), metricReceiverWrapper); } - // For testing - DockerImpl(DockerClient dockerClient) { - this(null, null); + DockerImpl(DockerClient dockerClient, MetricReceiverWrapper metricReceiver) { this.dockerClient = dockerClient; + this.dockerImageGC = new DockerImageGarbageCollector(this); + + Dimensions dimensions = new Dimensions.Builder().add("role", "docker").build(); + numberOfDockerDaemonFails = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "daemon.api_fails"); } @Override public void start() { - if (started) return; - started = true; - - if (config != null) { - dockerClient = createDockerClient(config); - - if (!config.networkNATed()) { - try { - setupDockerNetworkIfNeeded(); - } catch (Exception e) { - throw new DockerException("Could not setup docker network", e); - } - } + try { + setupDockerNetworkIfNeeded(); + } catch (Exception e) { + throw new DockerException("Could not setup docker network", e); } } @@ -306,7 +285,7 @@ public class DockerImpl implements Docker { @Override public void stopContainer(ContainerName containerName) { try { - dockerClient.stopContainerCmd(containerName.asString()).withTimeout(secondsToWaitBeforeKilling).exec(); + dockerClient.stopContainerCmd(containerName.asString()).withTimeout(SECONDS_TO_WAIT_BEFORE_KILLING).exec(); } catch (NotFoundException e) { throw new ContainerNotFoundException(containerName); } catch (NotModifiedException ignored) { @@ -320,11 +299,6 @@ public class DockerImpl implements Docker { @Override public void deleteContainer(ContainerName containerName) { try { - dockerImageGC.ifPresent(imageGC -> { - Optional<InspectContainerResponse> inspectResponse = inspectContainerCmd(containerName.asString()); - inspectResponse.ifPresent(response -> imageGC.updateLastUsedTimeFor(response.getImageId())); - }); - dockerClient.removeContainerCmd(containerName.asString()).exec(); } catch (NotFoundException e) { throw new ContainerNotFoundException(containerName); @@ -373,7 +347,7 @@ public class DockerImpl implements Docker { return encodedContainerName.substring(FRAMEWORK_CONTAINER_PREFIX.length()); } - private List<com.github.dockerjava.api.model.Container> listAllContainers() { + List<com.github.dockerjava.api.model.Container> listAllContainers() { try { return dockerClient.listContainersCmd().withShowAll(true).exec(); } catch (RuntimeException e) { @@ -382,7 +356,7 @@ public class DockerImpl implements Docker { } } - private List<Image> listAllImages() { + List<Image> listAllImages() { try { return dockerClient.listImagesCmd().withShowAll(true).exec(); } catch (RuntimeException e) { @@ -391,7 +365,7 @@ public class DockerImpl implements Docker { } } - private void deleteImage(final DockerImage dockerImage) { + void deleteImage(DockerImage dockerImage) { try { dockerClient.removeImageCmd(dockerImage.asString()).exec(); } catch (NotFoundException ignored) { @@ -403,13 +377,8 @@ public class DockerImpl implements Docker { } @Override - public void deleteUnusedDockerImages() { - if (!dockerImageGC.isPresent()) return; - - List<Image> images = listAllImages(); - List<com.github.dockerjava.api.model.Container> containers = listAllContainers(); - - dockerImageGC.get().getUnusedDockerImages(images, containers).forEach(this::deleteImage); + public boolean deleteUnusedDockerImages(List<DockerImage> excludes, Duration minImageAgeToDelete) { + return dockerImageGC.deleteUnusedDockerImages(excludes, minImageAgeToDelete); } private class ImagePullCallback extends PullImageResultCallback { @@ -428,10 +397,8 @@ public class DockerImpl implements Docker { @Override public void onComplete() { - Optional<InspectImageResponse> image = inspectImage(dockerImage); - if (image.isPresent()) { // Download successful, update image GC with the newly downloaded image + if (imageIsDownloaded(dockerImage)) { logger.log(LogLevel.INFO, "Download completed: " + dockerImage.asString()); - dockerImageGC.ifPresent(imageGC -> imageGC.updateLastUsedTimeFor(image.get().getId())); removeScheduledPoll(dockerImage); } else { throw new DockerClientException("Could not download image: " + dockerImage); @@ -476,9 +443,4 @@ public class DockerImpl implements Docker { return DockerClientImpl.getInstance(dockerClientConfig) .withDockerCmdExecFactory(dockerFactory); } - - void setMetrics(MetricReceiverWrapper metricReceiver) { - Dimensions dimensions = new Dimensions.Builder().add("role", "docker").build(); - numberOfDockerDaemonFails = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "daemon.api_fails"); - } } diff --git a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java index 13ff45808cf..f1a8d4ef65e 100644 --- a/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java +++ b/docker-api/src/test/java/com/yahoo/vespa/hosted/dockerapi/DockerImplTest.java @@ -33,6 +33,10 @@ import static org.mockito.Mockito.when; */ public class DockerImplTest { + private final DockerClient dockerClient = mock(DockerClient.class); + private final MetricReceiverWrapper metricReceiver = new MetricReceiverWrapper(MetricReceiver.nullImplementation); + private final DockerImpl docker = new DockerImpl(dockerClient, metricReceiver); + @Test public void testExecuteCompletes() { final String containerId = "container-id"; @@ -40,8 +44,6 @@ public class DockerImplTest { final String execId = "exec-id"; final int exitCode = 3; - final DockerClient dockerClient = mock(DockerClient.class); - final ExecCreateCmdResponse response = mock(ExecCreateCmdResponse.class); when(response.getId()).thenReturn(execId); @@ -64,7 +66,6 @@ public class DockerImplTest { when(state.isRunning()).thenReturn(false); when(state.getExitCode()).thenReturn(exitCode); - final Docker docker = new DockerImpl(dockerClient); final ProcessResult result = docker.executeInContainer(new ContainerName(containerId), command); assertThat(result.getExitStatus(), is(exitCode)); } @@ -87,12 +88,9 @@ public class DockerImplTest { PullImageCmd pullImageCmd = mock(PullImageCmd.class); when(pullImageCmd.exec(resultCallback.capture())).thenReturn(null); - final DockerClient dockerClient = mock(DockerClient.class); when(dockerClient.inspectImageCmd(image.asString())).thenReturn(imageInspectCmd); when(dockerClient.pullImageCmd(eq(image.asString()))).thenReturn(pullImageCmd); - final DockerImpl docker = new DockerImpl(dockerClient); - docker.setMetrics(new MetricReceiverWrapper(MetricReceiver.nullImplementation)); assertTrue("Should return true, we just scheduled the pull", docker.pullImageAsyncIfNeeded(image)); assertTrue("Should return true, the pull i still ongoing", docker.pullImageAsyncIfNeeded(image)); @@ -114,12 +112,9 @@ public class DockerImplTest { PullImageCmd pullImageCmd = mock(PullImageCmd.class); when(pullImageCmd.exec(resultCallback.capture())).thenReturn(null); - final DockerClient dockerClient = mock(DockerClient.class); when(dockerClient.inspectImageCmd(image.asString())).thenReturn(imageInspectCmd); when(dockerClient.pullImageCmd(eq(image.asString()))).thenReturn(pullImageCmd); - final DockerImpl docker = new DockerImpl(dockerClient); - docker.setMetrics(new MetricReceiverWrapper(MetricReceiver.nullImplementation)); assertTrue("Should return true, we just scheduled the pull", docker.pullImageAsyncIfNeeded(image)); assertTrue("Should return true, the pull i still ongoing", docker.pullImageAsyncIfNeeded(image)); |