Skip to content

Introduce InspectContainerResponse.Mounts #392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcuslinke merged 4 commits into from
Dec 8, 2015
Merged

Introduce InspectContainerResponse.Mounts #392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61bcaa7
Introduce InspectContainerResponse.Mounts Fix issue #388
Dec 7, 2015
c2d62fa
Code cleanup
Dec 7, 2015
089bdd1
Adding @CheckForNull
Dec 7, 2015
e043190
Use custom matcher for testing containers mounted volumes
Dec 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions src/main/java/com/github/dockerjava/api/command/InspectContainerResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.Map;

import com.github.dockerjava.core.RemoteApiVersion;

import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;
Expand All @@ -14,6 +15,7 @@
import com.github.dockerjava.api.model.ContainerConfig;
import com.github.dockerjava.api.model.HostConfig;
import com.github.dockerjava.api.model.Ports;
import com.github.dockerjava.api.model.Volume;
import com.github.dockerjava.api.model.VolumeBind;
import com.github.dockerjava.api.model.VolumeBinds;
import com.github.dockerjava.api.model.VolumeRW;
Expand Down Expand Up @@ -89,6 +91,9 @@ public class InspectContainerResponse {
@JsonProperty("VolumesRW")
private VolumesRW volumesRW;

@JsonProperty("Mounts")
private List<Mount> mounts;
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mount class itself may appear in any time, but field appeared @since 1.20 ?

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Field 'Mounts' exists since 1.20

Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you javadoc it? :) Better mark versions while you know them, because later it will be difficult to compare all of them
screenshot 2015-11-20 20 19 29

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know... Will do :)

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to the getter


public String getId() {
return id;
}
Expand Down Expand Up @@ -134,7 +139,13 @@ public VolumeBind[] getVolumes() {
return volumes == null ? null : volumes.getBinds();
}

/**
* @deprecated As of {@link RemoteApiVersion#VERSION_1_20}
* use {@link #getMounts()} instead
*/
@JsonIgnore
@Deprecated
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it be not available in JSON response or it just deprecated by docker and fill be removed in future?

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VolumesRW field is removed with 1.20. This may lead to NullpointerExceptions, so adding @CheckForNull now.

@CheckForNull
public VolumeRW[] getVolumesRW() {
return volumesRW == null ? null : volumesRW.getVolumesRW();
}
Expand Down Expand Up @@ -167,6 +178,14 @@ public String getMountLabel() {
return mountLabel;
}

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
public List<Mount> getMounts() {
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckForNull? on < 1.20 expected to be null

return mounts;
}

public List<String> getExecIds() {
return execIds;
}
Expand Down Expand Up @@ -413,4 +432,81 @@ public String toString() {
}
}

@JsonIgnoreProperties(ignoreUnknown = true)
public static class Mount {

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
@JsonProperty("Name")
private String name;

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
@JsonProperty("Source")
private String source;

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
@JsonProperty("Destination")
private Volume destination;

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
@JsonProperty("Driver")
private String driver;

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
@JsonProperty("Mode")
private String mode;

/**
* @since {@link RemoteApiVersion#VERSION_1_20}
*/
@CheckForNull
@JsonProperty("RW")
private Boolean rw;

@CheckForNull
public String getName() {
return name;
}

@CheckForNull
public String getSource() {
return source;
}

@CheckForNull
public Volume getDestination() {
return destination;
}

@CheckForNull
public String getDriver() {
return driver;
}

@CheckForNull
public String getMode() {
return mode;
}

@CheckForNull
public Boolean getRW() {
return rw;
}

}

}
11 changes: 4 additions & 7 deletions src/main/java/com/github/dockerjava/api/model/Binds.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
Expand All @@ -17,7 +16,6 @@
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.node.NullNode;

@JsonSerialize(using = Binds.Serializer.class)
@JsonDeserialize(using = Binds.Deserializer.class)
Expand Down Expand Up @@ -58,12 +56,11 @@ public Binds deserialize(JsonParser jsonParser, DeserializationContext deseriali
List<Bind> binds = new ArrayList<Bind>();
ObjectCodec oc = jsonParser.getCodec();
JsonNode node = oc.readTree(jsonParser);
for (Iterator<Map.Entry<String, JsonNode>> it = node.fields(); it.hasNext();) {
for (Iterator<JsonNode> it = node.elements(); it.hasNext();) {

JsonNode field = it.next();
binds.add(Bind.parse(field.asText()));

Map.Entry<String, JsonNode> field = it.next();
if (!field.getValue().equals(NullNode.getInstance())) {
binds.add(Bind.parse(field.getKey()));
}
}
return new Binds(binds.toArray(new Bind[0]));
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/github/dockerjava/api/model/VolumeRW.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.node.NullNode;
import com.github.dockerjava.core.RemoteApiVersion;

/**
* Represents a bind mounted volume in a Docker container.
*
* @see Bind
* @deprecated since {@link RemoteApiVersion#VERSION_1_20}
*/
@JsonDeserialize(using = VolumeRW.Deserializer.class)
@JsonSerialize(using = VolumeRW.Serializer.class)
@Deprecated
public class VolumeRW {

private Volume volume;
Expand Down
38 changes: 21 additions & 17 deletions src/test/java/com/github/dockerjava/client/AbstractDockerClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.github.dockerjava.client;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;

import java.io.File;
import java.io.IOException;
Expand All @@ -15,14 +15,17 @@

import org.apache.commons.io.IOUtils;
import org.apache.commons.io.LineIterator;
import org.hamcrest.FeatureMatcher;
import org.hamcrest.Matcher;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.ITestResult;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.exception.DockerException;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.command.InspectContainerResponse.Mount;
import com.github.dockerjava.api.exception.DockerException;
import com.github.dockerjava.api.model.Frame;
import com.github.dockerjava.api.model.Volume;
import com.github.dockerjava.api.model.VolumeBind;
Expand All @@ -32,13 +35,12 @@
import com.github.dockerjava.core.command.BuildImageResultCallback;
import com.github.dockerjava.core.command.LogContainerResultCallback;
import com.github.dockerjava.core.command.PullImageResultCallback;
import com.google.common.base.Joiner;

public abstract class AbstractDockerClientTest extends Assert {

public static final Logger LOG = LoggerFactory.getLogger(AbstractDockerClientTest.class);

private String apiVersion = "1.19";
private String apiVersion = "1.21";
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be changed to RemoteApiVersion object?

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make sense. Will do as suggested.

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems thats not that simple. RemoteApiVersion is not used by DockerClientConfigBuilder so far. So leaving this for separate PR.


protected DockerClient dockerClient;

Expand Down Expand Up @@ -172,21 +174,23 @@ public static Boolean available(int port) {
return false;
}

/**
* Asserts that {@link InspectContainerResponse#getVolumes()} (<code>.Volumes</code>) has {@link VolumeBind}s for
* the given {@link Volume}s
*/
public static void assertContainerHasVolumes(InspectContainerResponse inspectContainerResponse,
Volume... expectedVolumes) {

List<Volume> volumes = new ArrayList<Volume>();
VolumeBind[] volumeBinds = inspectContainerResponse.getVolumes();
if (volumeBinds != null) {
for (VolumeBind bind : volumeBinds) {
volumes.add(new Volume(bind.getContainerPath()));
protected MountedVolumes mountedVolumes(Matcher<? super List<Volume>> subMatcher) {
return new MountedVolumes(subMatcher, "Mounted volumes", "mountedVolumes");
}

private static class MountedVolumes extends FeatureMatcher<InspectContainerResponse, List<Volume>> {
public MountedVolumes(Matcher<? super List<Volume>> subMatcher, String featureDescription, String featureName) {
super(subMatcher, featureDescription, featureName);
}

@Override
public List<Volume> featureValueOf(InspectContainerResponse item) {
List<Volume> volumes = new ArrayList<Volume>();
for (Mount mount : item.getMounts()) {
volumes.add(mount.getDestination());
}
return volumes;
}
assertThat(volumes, contains(expectedVolumes));
}

protected String containerLog(String containerId) throws Exception {
Expand Down
61 changes: 32 additions & 29 deletions src/test/java/com/github/dockerjava/core/command/CreateContainerCmdImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,5 @@
package com.github.dockerjava.core.command;

import com.github.dockerjava.api.exception.ConflictException;
import com.github.dockerjava.api.exception.DockerException;
import com.github.dockerjava.api.command.CreateContainerResponse;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.model.AccessMode;
import com.github.dockerjava.api.model.Bind;
import com.github.dockerjava.api.model.Device;
import com.github.dockerjava.api.model.ExposedPort;
import com.github.dockerjava.api.model.Link;
import com.github.dockerjava.api.model.LogConfig;
import com.github.dockerjava.api.model.Ports;
import com.github.dockerjava.api.model.RestartPolicy;
import com.github.dockerjava.api.model.Ulimit;
import com.github.dockerjava.api.model.Volume;
import com.github.dockerjava.api.model.VolumeRW;
import com.github.dockerjava.api.model.VolumesFrom;
import com.github.dockerjava.client.AbstractDockerClientTest;
import org.testng.ITestResult;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;

import static com.github.dockerjava.api.model.Capability.MKNOD;
import static com.github.dockerjava.api.model.Capability.NET_ADMIN;
import static org.hamcrest.MatcherAssert.assertThat;
Expand All @@ -45,7 +21,28 @@
import java.util.Map;
import java.util.UUID;

import static org.hamcrest.MatcherAssert.assertThat;
import org.testng.ITestResult;
import org.testng.annotations.AfterMethod;
Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the settings in your IDE about imports?
Usually static imports are after generic imports and javax/java.lang are usually in separate blocks.

Copy link
Copy Markdown
Member

@KostyaSha KostyaSha Dec 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just trying understand your styling and materialise later in checkstyle)

Copy link
Copy Markdown
Contributor Author

@marcuslinke marcuslinke Dec 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eclipse code formatting options doesn't have options for static import order AFAIK.

import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;

import com.github.dockerjava.api.command.CreateContainerResponse;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.exception.ConflictException;
import com.github.dockerjava.api.exception.DockerException;
import com.github.dockerjava.api.model.Bind;
import com.github.dockerjava.api.model.Device;
import com.github.dockerjava.api.model.ExposedPort;
import com.github.dockerjava.api.model.Link;
import com.github.dockerjava.api.model.LogConfig;
import com.github.dockerjava.api.model.Ports;
import com.github.dockerjava.api.model.RestartPolicy;
import com.github.dockerjava.api.model.Ulimit;
import com.github.dockerjava.api.model.Volume;
import com.github.dockerjava.api.model.VolumesFrom;
import com.github.dockerjava.client.AbstractDockerClientTest;

@Test(groups = "integration")
public class CreateContainerCmdImplTest extends AbstractDockerClientTest {
Expand Down Expand Up @@ -107,9 +104,12 @@ public void createContainerWithVolume() throws DockerException {

assertThat(inspectContainerResponse.getConfig().getVolumes().keySet(), contains("/var/log"));

assertThat(inspectContainerResponse.getVolumesRW(), hasItemInArray(new VolumeRW(volume, AccessMode.rw)));
assertThat(inspectContainerResponse.getMounts().get(0).getDestination(), equalTo(volume));
assertThat(inspectContainerResponse.getMounts().get(0).getMode(), equalTo(""));
assertThat(inspectContainerResponse.getMounts().get(0).getRW(), equalTo(true));
}


@Test
public void createContainerWithReadOnlyVolume() throws DockerException {

Expand All @@ -128,7 +128,9 @@ public void createContainerWithReadOnlyVolume() throws DockerException {

assertThat(inspectContainerResponse.getConfig().getVolumes().keySet(), contains("/srv/test"));

assertThat(Arrays.asList(inspectContainerResponse.getVolumesRW()), contains(new VolumeRW(volume)));
assertThat(inspectContainerResponse.getMounts().get(0).getDestination(), equalTo(volume));
// TODO: Create a read-only volume and test like this
// assertFalse(inspectContainerResponse.getMounts().get(0).getRW());
}

@Test
Expand All @@ -151,7 +153,7 @@ public void createContainerWithVolumesFrom() throws DockerException {
InspectContainerResponse inspectContainerResponse1 = dockerClient.inspectContainerCmd(container1.getId())
.exec();

assertContainerHasVolumes(inspectContainerResponse1, volume1, volume2);
assertThat(inspectContainerResponse1, mountedVolumes(containsInAnyOrder(volume1, volume2)));

// create a second container with volumes from first container
CreateContainerResponse container2 = dockerClient.createContainerCmd("busybox").withCmd("sleep", "9999")
Expand All @@ -177,7 +179,8 @@ public void createContainerWithVolumesFrom() throws DockerException {

assertThat(inspectContainerResponse2.getHostConfig().getVolumesFrom(), hasItemInArray(new VolumesFrom(
container1Name)));
assertContainerHasVolumes(inspectContainerResponse2, volume1, volume2);

assertThat(inspectContainerResponse2, mountedVolumes(containsInAnyOrder(volume1, volume2)));
}

@Test
Expand Down
Loading