JCLOUDS-997 Allow nullable docker configuration - the null has another meaning than empty list/map (e.g. CMD: null=default, emptyList=no-command)
Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/e21767db Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/e21767db Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/e21767db Branch: refs/heads/master Commit: e21767dbdebf54669a38b2ac609329d2b917dccb Parents: 3901403 Author: Josef Cacek <[email protected]> Authored: Thu Sep 17 18:07:04 2015 +0200 Committer: Ignasi Barrera <[email protected]> Committed: Fri Sep 18 10:28:37 2015 +0200 ---------------------------------------------------------------------- .../compute/options/DockerTemplateOptions.java | 57 +++++++++++++---- .../strategy/DockerComputeServiceAdapter.java | 13 +--- .../java/org/jclouds/docker/domain/Config.java | 25 ++++---- .../jclouds/docker/internal/NullSafeCopies.java | 40 +++++++++++- .../options/DockerTemplateOptionsTest.java | 9 ++- .../org/jclouds/docker/domain/ConfigTest.java | 65 ++++++++++++++++++++ 6 files changed, 172 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/e21767db/apis/docker/src/main/java/org/jclouds/docker/compute/options/DockerTemplateOptions.java ---------------------------------------------------------------------- diff --git a/apis/docker/src/main/java/org/jclouds/docker/compute/options/DockerTemplateOptions.java b/apis/docker/src/main/java/org/jclouds/docker/compute/options/DockerTemplateOptions.java index cce2b22..4088a54 100644 --- a/apis/docker/src/main/java/org/jclouds/docker/compute/options/DockerTemplateOptions.java +++ b/apis/docker/src/main/java/org/jclouds/docker/compute/options/DockerTemplateOptions.java @@ -24,6 +24,7 @@ import java.util.Map; import org.jclouds.compute.ComputeService; import org.jclouds.compute.options.TemplateOptions; +import org.jclouds.docker.internal.NullSafeCopies; import org.jclouds.domain.LoginCredentials; import org.jclouds.javax.annotation.Nullable; import org.jclouds.scriptbuilder.domain.Statement; @@ -54,9 +55,10 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable protected String hostname; protected Integer memory; protected Integer cpuShares; - protected List<String> commands = ImmutableList.of(); + protected List<String> entrypoint; + protected List<String> commands; protected Map<String, String> volumes = ImmutableMap.of(); - protected List<String> env = ImmutableList.of(); + protected List<String> env; protected Map<Integer, Integer> portBindings = ImmutableMap.of(); protected String networkMode; protected Map<String, String> extraHosts = ImmutableMap.of(); @@ -82,12 +84,9 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable } eTo.memory(memory); eTo.cpuShares(cpuShares); - if (!commands.isEmpty()) { - eTo.commands(commands); - } - if (!env.isEmpty()) { - eTo.env(env); - } + eTo.entrypoint(entrypoint); + eTo.commands(commands); + eTo.env(env); if (!portBindings.isEmpty()) { eTo.portBindings(portBindings); } @@ -109,6 +108,7 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable equal(this.hostname, that.hostname) && equal(this.dns, that.dns) && equal(this.memory, that.memory) && + equal(this.entrypoint, that.entrypoint) && equal(this.commands, that.commands) && equal(this.cpuShares, that.cpuShares) && equal(this.env, that.env) && @@ -118,7 +118,7 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable @Override public int hashCode() { - return Objects.hashCode(super.hashCode(), volumes, hostname, dns, memory, commands, cpuShares, env, portBindings, extraHosts); + return Objects.hashCode(super.hashCode(), volumes, hostname, dns, memory, entrypoint, commands, cpuShares, env, portBindings, extraHosts); } @Override @@ -128,6 +128,7 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable .add("hostname", hostname) .add("memory", memory) .add("cpuShares", cpuShares) + .add("entrypoint", entrypoint) .add("commands", commands) .add("volumes", volumes) .add("env", env) @@ -160,13 +161,24 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable return this; } + public DockerTemplateOptions entrypoint(Iterable<String> entrypoint) { + this.entrypoint = NullSafeCopies.copyWithNullOf(entrypoint); + return this; + } + + public DockerTemplateOptions entrypoint(String... entrypoint) { + this.entrypoint = NullSafeCopies.copyWithNullOf(entrypoint); + return this; + } + public DockerTemplateOptions commands(Iterable<String> commands) { - this.commands = ImmutableList.copyOf(checkNotNull(commands, "commands")); + this.commands = NullSafeCopies.copyWithNullOf(commands); return this; } public DockerTemplateOptions commands(String...commands) { - return commands(ImmutableList.copyOf(checkNotNull(commands, "commands"))); + this.commands = NullSafeCopies.copyWithNullOf(commands); + return this; } public DockerTemplateOptions cpuShares(@Nullable Integer cpuShares) { @@ -175,12 +187,13 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable } public DockerTemplateOptions env(Iterable<String> env) { - this.env = ImmutableList.copyOf(checkNotNull(env, "env")); + this.env = NullSafeCopies.copyWithNullOf(env); return this; } public DockerTemplateOptions env(String...env) { - return env(ImmutableList.copyOf(checkNotNull(env, "env"))); + this.env = NullSafeCopies.copyWithNullOf(env); + return this; } /** @@ -230,6 +243,8 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable public Integer getMemory() { return memory; } + public List<String> getEntrypoint() { return entrypoint; } + public List<String> getCommands() { return commands; } public Integer getCpuShares() { return cpuShares; } @@ -285,6 +300,22 @@ public class DockerTemplateOptions extends TemplateOptions implements Cloneable } /** + * @see DockerTemplateOptions#entrypoint(String...) + */ + public static DockerTemplateOptions entrypoint(String...entrypoint) { + DockerTemplateOptions options = new DockerTemplateOptions(); + return options.entrypoint(entrypoint); + } + + /** + * @see DockerTemplateOptions#entrypoint(Iterable) + */ + public static DockerTemplateOptions entrypoint(Iterable<String> entrypoint) { + DockerTemplateOptions options = new DockerTemplateOptions(); + return options.entrypoint(entrypoint); + } + + /** * @see DockerTemplateOptions#commands(String...) */ public static DockerTemplateOptions commands(String...commands) { http://git-wip-us.apache.org/repos/asf/jclouds/blob/e21767db/apis/docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java ---------------------------------------------------------------------- diff --git a/apis/docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java b/apis/docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java index 62ea752..b1041d0 100644 --- a/apis/docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java +++ b/apis/docker/src/main/java/org/jclouds/docker/compute/strategy/DockerComputeServiceAdapter.java @@ -97,19 +97,12 @@ public class DockerComputeServiceAdapter implements .image(imageId) .exposedPorts(exposedPorts); - if (!templateOptions.getCommands().isEmpty()) { - containerConfigBuilder.cmd(templateOptions.getCommands()); - } - + containerConfigBuilder.entrypoint(templateOptions.getEntrypoint()); + containerConfigBuilder.cmd(templateOptions.getCommands()); containerConfigBuilder.memory(templateOptions.getMemory()); - containerConfigBuilder.hostname(templateOptions.getHostname()); - containerConfigBuilder.cpuShares(templateOptions.getCpuShares()); - - if (!templateOptions.getEnv().isEmpty()) { - containerConfigBuilder.env(templateOptions.getEnv()); - } + containerConfigBuilder.env(templateOptions.getEnv()); if (!templateOptions.getVolumes().isEmpty()) { Map<String, Object> volumes = Maps.newLinkedHashMap(); http://git-wip-us.apache.org/repos/asf/jclouds/blob/e21767db/apis/docker/src/main/java/org/jclouds/docker/domain/Config.java ---------------------------------------------------------------------- diff --git a/apis/docker/src/main/java/org/jclouds/docker/domain/Config.java b/apis/docker/src/main/java/org/jclouds/docker/domain/Config.java index b16f4b5..2203a85 100644 --- a/apis/docker/src/main/java/org/jclouds/docker/domain/Config.java +++ b/apis/docker/src/main/java/org/jclouds/docker/domain/Config.java @@ -18,6 +18,7 @@ package org.jclouds.docker.domain; import static com.google.common.base.Preconditions.checkNotNull; import static org.jclouds.docker.internal.NullSafeCopies.copyOf; +import static org.jclouds.docker.internal.NullSafeCopies.copyWithNullOf; import java.util.List; import java.util.Map; @@ -55,11 +56,11 @@ public abstract class Config { public abstract boolean stdinOnce(); - public abstract List<String> env(); + @Nullable public abstract List<String> env(); - public abstract List<String> cmd(); + @Nullable public abstract List<String> cmd(); - public abstract List<String> entrypoint(); + @Nullable public abstract List<String> entrypoint(); public abstract String image(); @@ -119,10 +120,11 @@ public abstract class Config { boolean publishAllPorts, boolean privileged, List<String> dns, String dnsSearch, String volumesFrom, List<String> capAdd, List<String> capDrop, Map<String, String> restartPolicy) { return new AutoValue_Config(hostname, domainname, user, memory, memorySwap, cpuShares, attachStdin, - attachStdout, attachStderr, tty, openStdin, stdinOnce, copyOf(env), copyOf(cmd), copyOf(entrypoint), - image, copyOf(volumes), workingDir, networkDisabled, copyOf(exposedPorts), copyOf(securityOpts), hostConfig, + attachStdout, attachStderr, tty, openStdin, stdinOnce, copyWithNullOf(env), copyWithNullOf(cmd), + copyWithNullOf(entrypoint), image, copyOf(volumes), workingDir, networkDisabled, + copyOf(exposedPorts), copyOf(securityOpts), hostConfig, copyOf(binds), copyOf(links), copyOf(lxcConf), copyOf(portBindings), publishAllPorts, privileged, - copyOf(dns), dnsSearch, volumesFrom, copyOf(capAdd), copyOf(capDrop), copyOf(restartPolicy)); + copyWithNullOf(dns), dnsSearch, volumesFrom, copyOf(capAdd), copyOf(capDrop), copyOf(restartPolicy)); } public static Builder builder() { @@ -146,9 +148,9 @@ public abstract class Config { private boolean tty; private boolean openStdin; private boolean stdinOnce; - private List<String> env = Lists.newArrayList(); - private List<String> cmd = Lists.newArrayList(); - private List<String> entrypoint = Lists.newArrayList(); + private List<String> env; + private List<String> cmd; + private List<String> entrypoint; private String image; private Map<String, ?> volumes = Maps.newHashMap(); private String workingDir; @@ -281,7 +283,7 @@ public abstract class Config { } public Builder hostConfig(HostConfig hostConfig) { - this.hostConfig = checkNotNull(hostConfig, "hostConfig"); + this.hostConfig = hostConfig; return this; } @@ -356,7 +358,8 @@ public abstract class Config { return hostname(in.hostname()).domainname(in.domainname()).user(in.user()).memory(in.memory()) .memorySwap(in.memorySwap()).cpuShares(in.cpuShares()).attachStdin(in.attachStdin()) .attachStdout(in.attachStdout()).attachStderr(in.attachStderr()).tty(in.tty()) - .image(in.image()).volumes(in.volumes()).workingDir(in.workingDir()) + .openStdin(in.openStdin()).stdinOnce(in.stdinOnce()).env(in.env()).cmd(in.cmd()) + .entrypoint(in.entrypoint()).image(in.image()).volumes(in.volumes()).workingDir(in.workingDir()) .networkDisabled(in.networkDisabled()).exposedPorts(in.exposedPorts()).securityOpts(in.securityOpts()) .hostConfig(in.hostConfig()).binds(in.binds()).links(in.links()).lxcConf(in.lxcConf()) .portBindings(in.portBindings()).publishAllPorts(in.publishAllPorts()).privileged(in.privileged()) http://git-wip-us.apache.org/repos/asf/jclouds/blob/e21767db/apis/docker/src/main/java/org/jclouds/docker/internal/NullSafeCopies.java ---------------------------------------------------------------------- diff --git a/apis/docker/src/main/java/org/jclouds/docker/internal/NullSafeCopies.java b/apis/docker/src/main/java/org/jclouds/docker/internal/NullSafeCopies.java index c02d449..ad86bbb 100644 --- a/apis/docker/src/main/java/org/jclouds/docker/internal/NullSafeCopies.java +++ b/apis/docker/src/main/java/org/jclouds/docker/internal/NullSafeCopies.java @@ -27,11 +27,47 @@ import com.google.common.collect.ImmutableMap; public class NullSafeCopies { public static <K, V> Map<K, V> copyOf(@Nullable Map<K, V> map) { - return map != null ? ImmutableMap.copyOf(map) : ImmutableMap.<K, V>of(); + return map != null ? ImmutableMap.copyOf(map) : ImmutableMap.<K, V> of(); } public static <E> List<E> copyOf(@Nullable List<E> list) { - return list != null ? ImmutableList.copyOf(list) : ImmutableList.<E>of(); + return list != null ? ImmutableList.copyOf(list) : ImmutableList.<E> of(); + } + + /** + * Copies given List with keeping null value if provided. + * + * @param list + * instance to copy (maybe <code>null</code>) + * @return if the parameter is not-<code>null</code> then immutable copy; + * <code>null</code> otherwise + */ + public static <E> List<E> copyWithNullOf(@Nullable List<E> list) { + return list != null ? ImmutableList.copyOf(list) : null; + } + + /** + * Copies given {@link Iterable} into immutable {@link List} with keeping null value if provided. + * + * @param iterable + * instance to copy (maybe <code>null</code>) + * @return if the parameter is not-<code>null</code> then immutable copy; + * <code>null</code> otherwise + */ + public static <E> List<E> copyWithNullOf(@Nullable Iterable<E> iterable) { + return iterable != null ? ImmutableList.copyOf(iterable) : null; + } + + /** + * Copies given array into immutable {@link List} with keeping null value if provided. + * + * @param array + * instance to copy (maybe <code>null</code>) + * @return if the parameter is not-<code>null</code> then immutable copy; + * <code>null</code> otherwise + */ + public static <E> List<E> copyWithNullOf(@Nullable E[] array) { + return array != null ? ImmutableList.copyOf(array) : null; } private NullSafeCopies() { http://git-wip-us.apache.org/repos/asf/jclouds/blob/e21767db/apis/docker/src/test/java/org/jclouds/docker/compute/options/DockerTemplateOptionsTest.java ---------------------------------------------------------------------- diff --git a/apis/docker/src/test/java/org/jclouds/docker/compute/options/DockerTemplateOptionsTest.java b/apis/docker/src/test/java/org/jclouds/docker/compute/options/DockerTemplateOptionsTest.java index d20019a..a1145f5 100644 --- a/apis/docker/src/test/java/org/jclouds/docker/compute/options/DockerTemplateOptionsTest.java +++ b/apis/docker/src/test/java/org/jclouds/docker/compute/options/DockerTemplateOptionsTest.java @@ -61,6 +61,12 @@ public class DockerTemplateOptionsTest { } @Test + public void testEntrypoint() { + TemplateOptions options = DockerTemplateOptions.Builder.entrypoint("/bin/sh", "-c"); + assertEquals(options.as(DockerTemplateOptions.class).getEntrypoint(), ImmutableList.of("/bin/sh", "-c")); + } + + @Test public void testCommands() { TemplateOptions options = DockerTemplateOptions.Builder.commands("chmod 666 /etc/*", "rm -rf /var/run"); assertEquals(options.as(DockerTemplateOptions.class).getCommands(), ImmutableList.of("chmod 666 /etc/*", "rm -rf /var/run")); @@ -104,7 +110,8 @@ public class DockerTemplateOptionsTest { DockerTemplateOptions options = DockerTemplateOptions.Builder .memory(512) .cpuShares(4) - .commands("test") + .entrypoint("entry", "point") + .commands("test", "abc") .portBindings( ImmutableMap.<Integer, Integer> builder() .put(8443, 443).build()) http://git-wip-us.apache.org/repos/asf/jclouds/blob/e21767db/apis/docker/src/test/java/org/jclouds/docker/domain/ConfigTest.java ---------------------------------------------------------------------- diff --git a/apis/docker/src/test/java/org/jclouds/docker/domain/ConfigTest.java b/apis/docker/src/test/java/org/jclouds/docker/domain/ConfigTest.java new file mode 100644 index 0000000..0eef24d --- /dev/null +++ b/apis/docker/src/test/java/org/jclouds/docker/domain/ConfigTest.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jclouds.docker.domain; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +@Test(groups = "unit", testName = "ConfigTest") +public class ConfigTest { + + @Test + public void testFromConfig() { + Config origConfig = Config.builder() + .hostname("6c9932f478bd") + .env(ImmutableList.of("PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin")) + .image("57e570db16baba1e8c0d6f3c15868ddb400f64ff76ec948e65c3ca3f15fb3587") + .domainname("") + .user("") + .cmd(ImmutableList.of("-name", "7a:63:a2:39:7b:0f")) + .entrypoint(ImmutableList.of("/home/weave/weaver", "-iface", "ethwe", "-wait", "5")) + .image("zettio/weave") + .workingDir("/home/weave") + .exposedPorts(ImmutableMap.of("6783/tcp", ImmutableMap.of(), "6783/udp", ImmutableMap.of())) + .build(); + Config newConfig = Config.builder().fromConfig(origConfig).build(); + assertThat(origConfig).isEqualTo(newConfig); + } + + + @Test + public void testNullValuesPropagation() { + Config config = Config.builder() + .image("zettio/weave") + .build(); + + assertThat(config.cmd()).isNull(); + assertThat(config.entrypoint()).isNull(); + assertThat(config.env()).isNull(); + assertThat(config.hostname()).isNull(); + assertThat(config.domainname()).isNull(); + assertThat(config.workingDir()).isNull(); + assertThat(config.hostConfig()).isNull(); + assertThat(config.dns()).isNull(); + assertThat(config.dnsSearch()).isNull(); + assertThat(config.volumesFrom()).isNull(); + } +}
