Skip to content

Commit

Permalink
#513 : Changed getByName() to no use listContainers()
Browse files Browse the repository at this point in the history
Apparently this works for every supported version down to 1.18 (Docker 1.6), since `/containers/<id or name>/json` can use both id and name. We used it only for ids so fart, but this is changed with this commit. Also there is now DockerAccess.getServerApiCVersion() to detect the API version running on the server.
  • Loading branch information
rhuss committed Jul 13, 2016
1 parent 1067e63 commit 1438879
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 29 deletions.
8 changes: 8 additions & 0 deletions src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context

protected Logger log;

// API version as requested from the client
private String serverVersion;

/**
* Entry point for this plugin. It will set up the helper class and then calls
* {@link #executeInternal(ServiceHub)}
Expand Down Expand Up @@ -277,6 +280,11 @@ private DockerAccess createDockerAccess(String minimalVersion) throws MojoExecut
log);
access.start();
setDockerHostAddressProperty(dockerUrl);
serverVersion = access.getServerApiVersion();
if (EnvUtil.extractLargerVersion(version,serverVersion).equals(version)) {
throw new MojoExecutionException(
String.format("Server API version %s is smaller than request API version %s",serverVersion,version));
}
}
catch (IOException e) {
throw new MojoExecutionException("Cannot create docker access object ", e);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/fabric8/maven/docker/StartMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private void waitIfRequested(ServiceHub hub, ImageConfiguration imageConfig,

if (wait.getTcp() != null) {
try {
Container container = hub.getDockerAccess().inspectContainer(containerId);
Container container = hub.getQueryService().getMandatoryContainer(containerId);
checkers.add(getTcpWaitChecker(container, imageConfig.getDescription(), projectProperties, wait.getTcp(), logOut));
} catch (DockerAccessException e) {
throw new MojoExecutionException("Unable to access container.", e);
Expand Down Expand Up @@ -360,7 +360,7 @@ protected boolean showLogs(ImageConfiguration imageConfig) {
private void exposeContainerProps(QueryService queryService, String containerId, String alias)
throws DockerAccessException {
if (StringUtils.isNotEmpty(exposeContainerProps) && StringUtils.isNotEmpty(alias)) {
Container container = queryService.getContainer(containerId);
Container container = queryService.getMandatoryContainer(containerId);
Properties props = project.getProperties();
String prefix = addDot(exposeContainerProps) + addDot(alias);
props.put(prefix + "id", containerId);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/fabric8/maven/docker/StopMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private List<Container> getContainersToStop(QueryService queryService, ImageConf
List<Container> containers;
RunImageConfiguration.NamingStrategy strategy = image.getRunConfiguration().getNamingStrategy();
if (strategy == RunImageConfiguration.NamingStrategy.alias) {
Container container = queryService.getContainerByName(image.getAlias());
Container container = queryService.getContainer(image.getAlias());
if (container != null) {
containers = Collections.singletonList(container);
} else {
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/io/fabric8/maven/docker/access/DockerAccess.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,22 @@
*/
public interface DockerAccess {

/**
* Get the API version of the running server
*
* @return api version in the form "1.24"
* @throws DockerAccessException
*/
String getServerApiVersion() throws DockerAccessException;

/**
* Inspect a container
*
* @param containerId container id
* @return <code>ContainerDetails<code> representing the container
* @param containerIdOrName container id or name
* @return <code>ContainerDetails<code> representing the container or null if none could be found
* @throws DockerAccessException if the container could not be inspected
*/
Container inspectContainer(String containerId) throws DockerAccessException;
Container inspectContainer(String containerIdOrName) throws DockerAccessException;

/**
* Check whether the given name exists as image at the docker daemon
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/io/fabric8/maven/docker/access/UrlBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public String createContainer(String name) {
.build();
}

public String version() {
return String.format("%s/version", baseUrl);
}

public String deleteImage(String name, boolean force) {
return u("images/%s", name)
.p("force", force)
Expand Down Expand Up @@ -148,6 +152,10 @@ public String removeNetwork(String id) {
.build();
}

public String getBaseUrl() {
return baseUrl;
}

// ============================================================================

@SuppressWarnings("deprecation")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ public DockerAccessWithHcClient(String apiVersion,
}
}

/** {@inheritDoc} */
public String getServerApiVersion() throws DockerAccessException {
try {
String url = urlBuilder.version();
String response = delegate.get(url, 200);
JSONObject info = new JSONObject(response);
return info.getString("ApiVersion");
} catch (Exception e) {
throw new DockerAccessException(e, "Cannot extract API version from server %s", urlBuilder.getBaseUrl());
}
}

@Override
public void startExecContainer(String containerId, LogOutputSpec outputSpec) throws DockerAccessException {
try {
Expand Down Expand Up @@ -235,13 +247,13 @@ public LogGetHandle getLogAsync(String containerId, LogCallback callback) {
}

@Override
public Container inspectContainer(String containerId) throws DockerAccessException {
public Container inspectContainer(String containerIdOrName) throws DockerAccessException {
try {
String url = urlBuilder.inspectContainer(containerId);
String url = urlBuilder.inspectContainer(containerIdOrName);
String response = delegate.get(url, HTTP_OK);
return new ContainerDetails(new JSONObject(response));
} catch (IOException e) {
throw new DockerAccessException(e, "Unable to retrieve container name for [%s]", containerId);
throw new DockerAccessException(e, "Unable to retrieve container name for [%s]", containerIdOrName);
}
}

Expand Down
29 changes: 14 additions & 15 deletions src/main/java/io/fabric8/maven/docker/service/QueryService.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,26 @@ public QueryService(DockerAccess docker) {
/**
* Get container by id
*
* @param containerId id
* @return container
* @throws DockerAccessException if an error occurs or no container with this id exists
* @param containerIdOrName container id or name
* @return container found
* @throws DockerAccessException if an error occurs or no container with this id or name exists
*/
public Container getContainer(String containerId) throws DockerAccessException {
return docker.inspectContainer(containerId);
public Container getMandatoryContainer(String containerIdOrName) throws DockerAccessException {
Container container = getContainer(containerIdOrName);
if (container == null) {
throw new DockerAccessException("Cannot find container %s", containerIdOrName);
}
return container;
}

/**
* Get a container running for a given container name.
* @param containerName name of container to lookup
* @param containerIdOrName name of container to lookup
* @return the container found or <code>null</code> if no container is available.
* @throws DockerAccessException in case of an remote error
*/
public Container getContainerByName(final String containerName) throws DockerAccessException {
for (Container el : docker.listContainers()) {
if (containerName.equals(el.getName())) {
return el;
}
}
return null;
public Container getContainer(final String containerIdOrName) throws DockerAccessException {
return docker.inspectContainer(containerIdOrName);
}

/**
Expand Down Expand Up @@ -87,7 +86,7 @@ public Set<Network> getNetworks() throws DockerAccessException {
* @throws DockerAccessException if access to the docker daemon fails
*/
public String getContainerName(String containerId) throws DockerAccessException {
return getContainer(containerId).getName();
return getMandatoryContainer(containerId).getName();
}

/**
Expand Down Expand Up @@ -153,7 +152,7 @@ public Container getLatestContainerForImage(String image) throws DockerAccessExc
* @throws DockerAccessException
*/
public boolean hasContainer(String containerName) throws DockerAccessException {
return getContainerByName(containerName) != null;
return getContainer(containerName) != null;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/io/fabric8/maven/docker/service/RunService.java
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ private void addVolumeConfig(ContainerHostConfig config, RunImageConfiguration r
}
}

List<String> findContainerIdsForLinks(List<String> links, boolean leaveUnresolvedIfNotFound) throws DockerAccessException {
private List<String> findContainerIdsForLinks(List<String> links, boolean leaveUnresolvedIfNotFound) throws DockerAccessException {
List<String> ret = new ArrayList<>();
for (String[] link : EnvUtil.splitOnLastColon(links)) {
String id = findContainerId(link[0], false);
Expand All @@ -331,7 +331,7 @@ List<String> findContainerIdsForLinks(List<String> links, boolean leaveUnresolve
}

// visible for testing
List<String> findVolumesFromContainers(List<String> images) throws DockerAccessException {
private List<String> findVolumesFromContainers(List<String> images) throws DockerAccessException {
List<String> list = new ArrayList<>();
if (images != null) {
for (String image : images) {
Expand Down Expand Up @@ -363,7 +363,7 @@ private String findContainerId(String imageNameOrAlias, boolean checkAllContaine

// check for external container. The image name is interpreted as a *container name* for that case ...
if (id == null) {
Container container = queryService.getContainerByName(imageNameOrAlias);
Container container = queryService.getContainer(imageNameOrAlias);
if (container != null && (checkAllContainers || container.isRunning())) {
id = container.getId();
}
Expand All @@ -378,7 +378,7 @@ private void startContainer(ImageConfiguration imageConfig, String id, PomLabel
}

private void updateMappedPortsAndAddresses(String containerId, PortMapping mappedPorts) throws DockerAccessException {
Container container = queryService.getContainer(containerId);
Container container = queryService.getMandatoryContainer(containerId);
if (container.isRunning()) {
mappedPorts.updateProperties(container.getPortBindings());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private boolean hasRequiredDependencies(Resolvable config) throws DockerAccessEx
for (String dependency : dependencies) {
// make sure the container exists, it's state will be verified elsewhere
if (processedImages.contains(dependency) ||
// Check also whether a *container* with this name exists in which case it is interpredted
// Check also whether a *container* with this name exists in which case it is interpreted
// as an external dependency which is already running
queryService.hasContainer(dependency)) {
continue;
Expand Down

0 comments on commit 1438879

Please sign in to comment.