Skip to content

Commit

Permalink
CHE-4197: Apply CHE_DOCKER_ALWAYS__PULL__IMAGE to container creation (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Mykola Morhun committed May 18, 2017
1 parent 9ba1c22 commit 064fdcb
Show file tree
Hide file tree
Showing 8 changed files with 260 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@
import org.eclipse.che.plugin.docker.client.params.InspectImageParams;
import org.eclipse.che.plugin.docker.client.params.KillContainerParams;
import org.eclipse.che.plugin.docker.client.params.ListContainersParams;
import org.eclipse.che.plugin.docker.client.params.ListImagesParams;
import org.eclipse.che.plugin.docker.client.params.PullParams;
import org.eclipse.che.plugin.docker.client.params.PushParams;
import org.eclipse.che.plugin.docker.client.params.PutResourceParams;
import org.eclipse.che.plugin.docker.client.params.RemoveContainerParams;
import org.eclipse.che.plugin.docker.client.params.RemoveImageParams;
import org.eclipse.che.plugin.docker.client.params.RemoveNetworkParams;
import org.eclipse.che.plugin.docker.client.params.network.RemoveNetworkParams;
import org.eclipse.che.plugin.docker.client.params.StartContainerParams;
import org.eclipse.che.plugin.docker.client.params.StartExecParams;
import org.eclipse.che.plugin.docker.client.params.StopContainerParams;
Expand Down Expand Up @@ -196,16 +197,34 @@ public Version getVersion() throws IOException {
}

/**
* Lists docker images.
* Lists all final layer docker images.
*
* @return list of docker images
* @throws IOException
* when a problem occurs with docker api calls
*/
public List<Image> listImages() throws IOException {
return listImages(ListImagesParams.create());
}

/**
* Lists docker images.
*
* @return list of docker images
* @throws IOException
* when a problem occurs with docker api calls
*/
public List<Image> listImages(ListImagesParams params) throws IOException {
final Filters filters = params.getFilters();

try (DockerConnection connection = connectionFactory.openConnection(dockerDaemonUri)
.method("GET")
.path(apiVersionPathPrefix + "/images/json")) {
addQueryParamIfNotNull(connection, "all", params.getAll());
addQueryParamIfNotNull(connection, "digests", params.getAll());
if (filters != null) {
connection.query("filters", urlPathSegmentEscaper().escape(toJson(filters.getFilters())));
}
final DockerResponse response = connection.request();
if (OK.getStatusCode() != response.getStatus()) {
throw getDockerException(response);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*******************************************************************************
* Copyright (c) 2012-2017 Codenvy, S.A.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Codenvy, S.A. - initial API and implementation
*******************************************************************************/
package org.eclipse.che.plugin.docker.client.params;

import org.eclipse.che.plugin.docker.client.json.Filters;

import java.util.Objects;

/**
* Arguments holder for {@link org.eclipse.che.plugin.docker.client.DockerConnector#listImages(ListImagesParams)}.
*
* @author Mykola Morhun
*/
public class ListImagesParams {

private Boolean all;
private Filters filters;
private Boolean digests;

/**
* Creates arguments holder.
*/
public static ListImagesParams create() {
return new ListImagesParams();
}

private ListImagesParams() {}

/**
* Adds all flag to this parameters.
*
* @param all
* if true show all images.
* Only images from a final layer (no children) are shown by default.
* @return this params instance
*/
public ListImagesParams withAll(Boolean all) {
this.all = all;
return this;
}

/**
* Adds filters to this parameters.
*
* @param filters
* Available filters:
* <ul>
* <li><code>before</code>=(<code>&lt;image-name&gt;[:&lt;tag&gt;]</code>, <code>&lt;image id&gt;</code> or <code>&lt;image@digest&gt;</code>)</li>
* <li><code>dangling=true</code></li>
* <li><code>label=key</code> or <code>label="key=value"</code> of an image label</li>
* <li><code>reference</code>=(<code>&lt;image-name&gt;[:&lt;tag&gt;]</code>)</li>
* <li><code>since</code>=(<code>&lt;image-name&gt;[:&lt;tag&gt;]</code>, <code>&lt;image id&gt;</code> or <code>&lt;image@digest&gt;</code>)</li>
* </ul>
* @return this params instance
*/
public ListImagesParams withFilters(Filters filters) {
this.filters = filters;
return this;
}

/**
* Adds all flag to this parameters.
*
* @param digests
* if true show digest information on each image
* @return this params instance
*/
public ListImagesParams withDigestst(Boolean digests) {
this.digests = digests;
return this;
}

public Boolean getAll() {
return all;
}

public Filters getFilters() {
return filters;
}

public Boolean getDigests() {
return digests;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ListImagesParams that = (ListImagesParams)o;
return Objects.equals(all, that.all) &&
Objects.equals(filters, that.filters) &&
Objects.equals(digests, that.digests);
}

@Override
public int hashCode() {
return Objects.hash(all, filters, digests);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* Contributors:
* Codenvy, S.A. - initial API and implementation
*******************************************************************************/
package org.eclipse.che.plugin.docker.client.params;
package org.eclipse.che.plugin.docker.client.params.network;

import javax.validation.constraints.NotNull;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,13 @@
import org.eclipse.che.plugin.docker.client.params.InspectImageParams;
import org.eclipse.che.plugin.docker.client.params.KillContainerParams;
import org.eclipse.che.plugin.docker.client.params.ListContainersParams;
import org.eclipse.che.plugin.docker.client.params.ListImagesParams;
import org.eclipse.che.plugin.docker.client.params.PullParams;
import org.eclipse.che.plugin.docker.client.params.PushParams;
import org.eclipse.che.plugin.docker.client.params.PutResourceParams;
import org.eclipse.che.plugin.docker.client.params.RemoveContainerParams;
import org.eclipse.che.plugin.docker.client.params.RemoveImageParams;
import org.eclipse.che.plugin.docker.client.params.RemoveNetworkParams;
import org.eclipse.che.plugin.docker.client.params.network.RemoveNetworkParams;
import org.eclipse.che.plugin.docker.client.params.StartContainerParams;
import org.eclipse.che.plugin.docker.client.params.StartExecParams;
import org.eclipse.che.plugin.docker.client.params.StopContainerParams;
Expand Down Expand Up @@ -294,13 +295,15 @@ public void shouldThrowDockerExceptionWhileGettingVersionIfResponseCodeIsNotSucc

@Test
public void shouldBeAbleToGetListImages() throws IOException, JsonParseException {
ListImagesParams listImagesParams = ListImagesParams.create();
List<Image> images = new ArrayList<>();
images.add(mock(Image.class));

doReturn(images).when(dockerConnector).parseResponseStreamAndClose(eq(inputStream),
Matchers.<TypeToken<List<Image>>>any());

List<Image> returnedImages =
dockerConnector.listImages();
dockerConnector.listImages(listImagesParams);

verify(dockerConnectionFactory).openConnection(any(URI.class));
verify(dockerConnection).method(REQUEST_METHOD_GET);
Expand All @@ -312,6 +315,23 @@ public void shouldBeAbleToGetListImages() throws IOException, JsonParseException
assertEquals(returnedImages, images);
}

@Test
public void shouldInvokeGetListImagesWithDefaultParamsWhenGetListImagesCalledWithoutParams() throws IOException, JsonParseException {
List<Image> images = new ArrayList<>();
images.add(mock(Image.class));

doReturn(images).when(dockerConnector).parseResponseStreamAndClose(eq(inputStream),
Matchers.<TypeToken<List<Image>>>any());

List<Image> returnedImages =
dockerConnector.listImages();

verify(dockerConnector).listImages((ListImagesParams)captor.capture());

assertEquals(captor.getValue(), ListImagesParams.create());
assertEquals(returnedImages, images);
}

@Test(expectedExceptions = DockerException.class, expectedExceptionsMessageRegExp = EXCEPTION_ERROR_MESSAGE)
public void shouldThrowDockerExceptionWhileListingImagesIfResponseCodeIsNotSuccess() throws IOException {
when(dockerResponse.getStatus()).thenReturn(RESPONSE_ERROR_CODE);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*******************************************************************************
* Copyright (c) 2012-2017 Codenvy, S.A.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Codenvy, S.A. - initial API and implementation
*******************************************************************************/
package org.eclipse.che.plugin.docker.client.params;

import org.eclipse.che.plugin.docker.client.json.Filters;
import org.testng.annotations.Test;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;

/**
* @author Mykola Morhun
*/
public class ListImagesParamsTest {

private static final boolean WITH_ALL = true;
private static final boolean WITH_DIGEST = true;
private static final Filters FILTERS = new Filters().withFilter("reference", "imageName");

private ListImagesParams listImagesParams;

@Test
public void shouldCreateParamsObjectWithAllPossibleParameters() {
listImagesParams = ListImagesParams.create()
.withAll(WITH_ALL)
.withDigestst(WITH_DIGEST)
.withFilters(FILTERS);

assertTrue(listImagesParams.getAll() == WITH_ALL);
assertTrue(listImagesParams.getDigests() == WITH_DIGEST);
assertEquals(listImagesParams.getFilters(), FILTERS);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*******************************************************************************/
package org.eclipse.che.plugin.docker.machine;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -53,6 +54,7 @@
import org.eclipse.che.plugin.docker.client.exception.NetworkNotFoundException;
import org.eclipse.che.plugin.docker.client.json.ContainerConfig;
import org.eclipse.che.plugin.docker.client.json.ContainerInfo;
import org.eclipse.che.plugin.docker.client.json.Filters;
import org.eclipse.che.plugin.docker.client.json.HostConfig;
import org.eclipse.che.plugin.docker.client.json.ImageConfig;
import org.eclipse.che.plugin.docker.client.json.PortBinding;
Expand All @@ -64,10 +66,11 @@
import org.eclipse.che.plugin.docker.client.params.BuildImageParams;
import org.eclipse.che.plugin.docker.client.params.CreateContainerParams;
import org.eclipse.che.plugin.docker.client.params.GetContainerLogsParams;
import org.eclipse.che.plugin.docker.client.params.ListImagesParams;
import org.eclipse.che.plugin.docker.client.params.PullParams;
import org.eclipse.che.plugin.docker.client.params.RemoveContainerParams;
import org.eclipse.che.plugin.docker.client.params.RemoveImageParams;
import org.eclipse.che.plugin.docker.client.params.RemoveNetworkParams;
import org.eclipse.che.plugin.docker.client.params.network.RemoveNetworkParams;
import org.eclipse.che.plugin.docker.client.params.StartContainerParams;
import org.eclipse.che.plugin.docker.client.params.TagParams;
import org.eclipse.che.plugin.docker.client.params.network.ConnectContainerToNetworkParams;
Expand Down Expand Up @@ -146,7 +149,7 @@ public class MachineProviderImpl implements MachineInstanceProvider {
private final DockerInstanceStopDetector dockerInstanceStopDetector;
private final RequestTransmitter transmitter;
private final JsonRpcEndpointToMachineNameHolder jsonRpcEndpointToMachineNameHolder;
private final boolean doForcePullOnBuild;
private final boolean doForcePullImage;
private final boolean privilegedMode;
private final int pidsLimit;
private final DockerMachineFactory dockerMachineFactory;
Expand Down Expand Up @@ -180,7 +183,7 @@ public MachineProviderImpl(DockerConnectorProvider dockerProvider,
@Named("machine.docker.machine_servers") Set<ServerConf> allMachinesServers,
@Named("machine.docker.dev_machine.machine_volumes") Set<String> devMachineSystemVolumes,
@Named("machine.docker.machine_volumes") Set<String> allMachinesSystemVolumes,
@Named("che.docker.always_pull_image") boolean doForcePullOnBuild,
@Named("che.docker.always_pull_image") boolean doForcePullImage,
@Named("che.docker.privileged") boolean privilegedMode,
@Named("che.docker.pids_limit") int pidsLimit,
@Named("machine.docker.dev_machine.machine_env") Set<String> devMachineEnvVariables,
Expand All @@ -203,7 +206,7 @@ public MachineProviderImpl(DockerConnectorProvider dockerProvider,
this.dockerMachineFactory = dockerMachineFactory;
this.dockerInstanceStopDetector = dockerInstanceStopDetector;
this.transmitter = transmitter;
this.doForcePullOnBuild = doForcePullOnBuild;
this.doForcePullImage = doForcePullImage;
this.privilegedMode = privilegedMode;
this.snapshotUseRegistry = snapshotUseRegistry;
// use-cases:
Expand Down Expand Up @@ -431,7 +434,7 @@ private String prepareImage(String machineName,

if (service.getBuild() != null && (service.getBuild().getContext() != null ||
service.getBuild().getDockerfileContent() != null)) {
buildImage(service, imageName, doForcePullOnBuild, progressMonitor);
buildImage(service, imageName, doForcePullImage, progressMonitor);
} else {
pullImage(service, imageName, progressMonitor);
}
Expand Down Expand Up @@ -466,9 +469,8 @@ protected void buildImage(CheServiceImpl service,
if (service.getBuild().getArgs() == null || service.getBuild().getArgs().isEmpty()) {
buildArgs = this.buildArgs;
} else {
buildArgs = new HashMap<>();
buildArgs = new HashMap<>(this.buildArgs);
buildArgs.putAll(service.getBuild().getArgs());
buildArgs.putAll(this.buildArgs);
}
buildImageParams.withForceRemoveIntermediateContainers(true)
.withRepository(machineImageName)
Expand Down Expand Up @@ -518,7 +520,8 @@ protected void pullImage(CheServiceImpl service,

try {
boolean isSnapshot = SNAPSHOT_LOCATION_PATTERN.matcher(dockerMachineSource.getLocation()).matches();
if (!isSnapshot || snapshotUseRegistry) {
boolean isImageExistLocally = isDockerImageExistLocally(dockerMachineSource.getRepository());
if ((!isSnapshot && (doForcePullImage || !isImageExistLocally)) || (isSnapshot && snapshotUseRegistry)) {
PullParams pullParams = PullParams.create(dockerMachineSource.getRepository())
.withTag(MoreObjects.firstNonNull(dockerMachineSource.getTag(),
LATEST_TAG))
Expand All @@ -544,6 +547,18 @@ protected void pullImage(CheServiceImpl service,
}
}

@VisibleForTesting
boolean isDockerImageExistLocally(String imageName) {
try {
return !docker.listImages(ListImagesParams.create()
.withFilters(new Filters().withFilter("reference", imageName)))
.isEmpty();
} catch (IOException e) {
LOG.warn("Failed to check image {} availability. Cause: {}", imageName, e.getMessage(), e);
return false; // consider that image doesn't exist locally
}
}

private String createContainer(String workspaceId,
String machineName,
boolean isDev,
Expand Down
Loading

0 comments on commit 064fdcb

Please sign in to comment.