fixed review comments

This commit is contained in:
petergardfjall 2012-12-10 13:56:21 +01:00
parent b4fd69287f
commit c1dd2b479d
5 changed files with 192 additions and 60 deletions

View File

@ -24,6 +24,7 @@ import static org.jclouds.compute.util.ComputeServiceUtils.metadataAndTagsAsComm
import static org.jclouds.concurrent.FutureIterables.transformParallel;
import java.util.Map;
import java.util.UUID;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ExecutorService;
@ -32,7 +33,6 @@ import java.util.concurrent.Future;
import javax.annotation.Resource;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;
import org.jclouds.Constants;
@ -92,18 +92,16 @@ public class GleSYSComputeServiceAdapter implements ComputeServiceAdapter<Server
private final ExecutorService userThreads;
private final Timeouts timeouts;
private final Supplier<Set<? extends Location>> locations;
private final Provider<String> passwordProvider;
@Inject
public GleSYSComputeServiceAdapter(GleSYSApi api, GleSYSAsyncApi aapi,
@Named(Constants.PROPERTY_USER_THREADS) ExecutorService userThreads, Timeouts timeouts,
@Memoized Supplier<Set<? extends Location>> locations, @Named("PASSWORD") Provider<String> passwordProvider) {
@Memoized Supplier<Set<? extends Location>> locations) {
this.api = checkNotNull(api, "api");
this.aapi = checkNotNull(aapi, "aapi");
this.userThreads = checkNotNull(userThreads, "userThreads");
this.timeouts = checkNotNull(timeouts, "timeouts");
this.locations = checkNotNull(locations, "locations");
this.passwordProvider = checkNotNull(passwordProvider, "passwordProvider");
}
@Override
@ -133,11 +131,12 @@ public class GleSYSComputeServiceAdapter implements ComputeServiceAdapter<Server
builder.memorySizeMB(template.getHardware().getRam());
builder.diskSizeGB(Math.round(template.getHardware().getVolumes().get(0).getSize()));
builder.cpuCores((int) template.getHardware().getProcessors().get(0).getCores());
builder.transferGB(50);// TODO: add to template options with default value
builder.transferGB(templateOptions.getTransferGB());
ServerSpec spec = builder.build();
String password = passwordProvider.get(); // TODO: add to templateOptions
// and set if present
// use random root password unless one was provided via template options
String password = templateOptions.hasRootPassword() ? templateOptions.getRootPassword() : getRandomPassword();
logger.debug(">> creating new Server spec(%s) name(%s) options(%s)", spec, name, createServerOptions);
ServerDetails result = api.getServerApi().createWithHostnameAndRootPassword(spec, name, password,
@ -148,6 +147,13 @@ public class GleSYSComputeServiceAdapter implements ComputeServiceAdapter<Server
.password(password).build());
}
/**
* @return a generated random password string
*/
private String getRandomPassword() {
return UUID.randomUUID().toString().replace("-","");
}
@Singleton
public static class FindLocationForServerSpec extends FindResourceInSet<ServerSpec, Location> {

View File

@ -18,12 +18,6 @@
*/
package org.jclouds.glesys.compute.config;
import java.util.UUID;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;
import org.jclouds.compute.ComputeServiceAdapter;
import org.jclouds.compute.config.ComputeServiceAdapterContextModule;
import org.jclouds.compute.domain.Hardware;
@ -42,9 +36,7 @@ import org.jclouds.glesys.domain.OSTemplate;
import org.jclouds.glesys.domain.ServerDetails;
import com.google.common.base.Function;
import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Names;
/**
*
@ -70,19 +62,7 @@ public class GleSYSComputeServiceContextModule extends
bind(new TypeLiteral<Function<String, OsFamilyVersion64Bit>>() {
}).to(ParseOsFamilyVersion64BitFromImageName.class);
bind(TemplateOptions.class).to(GleSYSTemplateOptions.class);
bind(String.class).annotatedWith(Names.named("PASSWORD")).toProvider(PasswordProvider.class).in(Scopes.SINGLETON);
// to have the compute service adapter override default locations
install(new LocationsFromComputeServiceAdapterModule<ServerDetails, Hardware, OSTemplate, String>(){});
}
@Named("PASSWORD")
@Singleton
public static class PasswordProvider implements Provider<String> {
@Override
public String get() {
return UUID.randomUUID().toString().replace("-","");
}
}
}

View File

@ -19,28 +19,32 @@
package org.jclouds.glesys.compute.options;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import java.util.Map;
import org.jclouds.compute.options.TemplateOptions;
import org.jclouds.glesys.domain.ServerSpec;
import com.google.common.base.Objects.ToStringHelper;
import com.google.common.net.InetAddresses;
/**
* Contains options supported by the
* {@link ComputeService#createNodesInGroup(String, int, TemplateOptions)} and
* {@link ComputeService#createNodesInGroup(String, int, TemplateOptions)} operations on the
* <em>glesys</em> provider.
* {@link ComputeService#createNodesInGroup(String, int, TemplateOptions)}
* operations on the <em>glesys</em> provider.
*
* <h2>Usage</h2> The recommended way to instantiate a {@link GleSYSTemplateOptions} object is to
* statically import {@code GleSYSTemplateOptions.*} and invoke a static creation method followed by
* an instance mutator (if needed):
* <h2>Usage</h2> The recommended way to instantiate a
* {@link GleSYSTemplateOptions} object is to statically import
* {@code GleSYSTemplateOptions.*} and invoke a static creation method followed
* by an instance mutator (if needed):
* <p>
*
* <pre>
* import static org.jclouds.compute.options.GleSYSTemplateOptions.Builder.*;
* ComputeService api = // get connection
* templateBuilder.options(inboundPorts(22, 80, 8080, 443));
* templateBuilder.options(rootPassword("caQu5rou"));
* Set&lt;? extends NodeMetadata&gt; set = api.createNodesInGroup(tag, 2, templateBuilder.build());
* </pre>
*
@ -48,7 +52,20 @@ import com.google.common.net.InetAddresses;
*/
public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable {
/**
* The IP address to assign to the new node instance. If set to "
* <code>any</code>" the node will be automatically assigned a free IP
* address.
*/
protected String ip = "any";
/**
* The password to set for the root user on the created server instance. If
* left unspecified, a random password will be assigned.
*/
protected String rootPassword = null;
/** The monthly data transfer limit (in GB) for the server. */
protected int transferGB = 50;
@Override
public GleSYSTemplateOptions clone() {
@ -61,39 +78,106 @@ public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable
public void copyTo(TemplateOptions to) {
super.copyTo(to);
if (to instanceof GleSYSTemplateOptions) {
GleSYSTemplateOptions eTo = GleSYSTemplateOptions.class.cast(to);
eTo.ip(ip);
GleSYSTemplateOptions copy = GleSYSTemplateOptions.class.cast(to);
copy.ip(ip);
copy.rootPassword(rootPassword);
copy.transferGB(transferGB);
}
}
/**
* Sets the IP address to assign to the new server instance. If set to "
* <code>any</code>" the server will be automatically assigned a free IP
* address.
*
* @see ServerApi#createWithHostnameAndRootPassword
* @see InetAddresses#isInetAddress
*/
public TemplateOptions ip(String ip) {
if (ip != null)
checkArgument("any".equals(ip) || InetAddresses.isInetAddress(ip), "ip %s is not valid", ip);
public GleSYSTemplateOptions ip(String ip) {
checkNotNull(ip);
checkArgument("any".equals(ip) || InetAddresses.isInetAddress(ip), "ip %s is not valid", ip);
this.ip = ip;
return this;
}
/**
* @return the IP address to assign to the new server instance.
*/
public String getIp() {
return ip;
}
public static final GleSYSTemplateOptions NONE = new GleSYSTemplateOptions();
/**
* Sets the password for the root user on the created server instance. If
* left unspecified, a random password will be assigned.
*
* @see ServerApi#createWithHostnameAndRootPassword
*/
public GleSYSTemplateOptions rootPassword(String rootPassword) {
checkNotNull(rootPassword, "root password cannot be null");
this.rootPassword = rootPassword;
return this;
}
/**
* @return the password set for the root user or <code>null</code> if none is
* set (and a random password will be assigned).
*/
public String getRootPassword() {
return rootPassword;
}
/**
* @return <code>true</code> if a root password has been specified.
*/
public boolean hasRootPassword() {
return rootPassword != null;
}
/**
* Sets the monthly data transfer limit (in GB) for the server.
*
* @see ServerSpec#getTransferGB()
*/
public GleSYSTemplateOptions transferGB(int transferGB) {
checkArgument(transferGB >= 0, "transferGB value must be >= 0", transferGB);
this.transferGB = transferGB;
return this;
}
/**
* @return the monthly data transfer limit (in GB) for the server.
*/
public int getTransferGB() {
return transferGB;
}
public static class Builder {
/**
* @see #ip
* @see GleSYSTemplateOptions#ip
*/
public static GleSYSTemplateOptions ip(String ip) {
GleSYSTemplateOptions options = new GleSYSTemplateOptions();
return GleSYSTemplateOptions.class.cast(options.ip(ip));
}
/**
* @see GleSYSTemplateOptions#rootPassword
*/
public static GleSYSTemplateOptions rootPassword(String rootPassword) {
GleSYSTemplateOptions options = new GleSYSTemplateOptions();
return GleSYSTemplateOptions.class.cast(options.rootPassword(rootPassword));
}
/**
* @see GleSYSTemplateOptions#transferGB
*/
public static GleSYSTemplateOptions transferGB(int transferGB) {
GleSYSTemplateOptions options = new GleSYSTemplateOptions();
return GleSYSTemplateOptions.class.cast(options.transferGB(transferGB));
}
// methods that only facilitate returning the correct object type
/**
@ -111,7 +195,7 @@ public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable
GleSYSTemplateOptions options = new GleSYSTemplateOptions();
return GleSYSTemplateOptions.class.cast(options.blockOnPort(port, seconds));
}
/**
* @see TemplateOptions#userMetadata(Map)
*/
@ -178,4 +262,18 @@ public class GleSYSTemplateOptions extends TemplateOptions implements Cloneable
public GleSYSTemplateOptions userMetadata(String key, String value) {
return GleSYSTemplateOptions.class.cast(super.userMetadata(key, value));
}
@Override
public ToStringHelper string() {
ToStringHelper stringHelper = super.string();
stringHelper.add("transferGB", this.transferGB);
stringHelper.add("ip", this.ip);
if (this.hasRootPassword()) {
stringHelper.add("rootPasswordPresent", true);
}
return stringHelper;
}
}

View File

@ -25,14 +25,12 @@ import org.jclouds.apis.ApiMetadata;
import org.jclouds.compute.ComputeService;
import org.jclouds.compute.ComputeServiceContext;
import org.jclouds.glesys.GleSYSApiMetadata;
import org.jclouds.glesys.compute.config.GleSYSComputeServiceContextModule.PasswordProvider;
import org.jclouds.http.HttpRequest;
import org.jclouds.http.HttpResponse;
import org.jclouds.rest.internal.BaseRestApiExpectTest;
import com.google.common.base.Function;
import com.google.common.collect.ImmutableMap;
import com.google.inject.AbstractModule;
import com.google.inject.Injector;
import com.google.inject.Module;
@ -57,15 +55,6 @@ public abstract class BaseGleSYSComputeServiceExpectTest extends BaseRestApiExpe
return createInjector(fn, module, props).getInstance(ComputeService.class);
}
protected PasswordProvider passwordGenerator() {
// make sure we can predict passwords generated for createServer requests
return new PasswordProvider() {
public String get() {
return "foo";
}
};
}
protected Injector injectorForKnownArgumentsAndConstantPassword() {
return injectorForKnownArgumentsAndConstantPassword(ImmutableMap.<HttpRequest, HttpResponse> of());
}
@ -95,14 +84,7 @@ public abstract class BaseGleSYSComputeServiceExpectTest extends BaseRestApiExpe
.addHeader("Authorization", "Basic aWRlbnRpdHk6Y3JlZGVudGlhbA==").build(),
HttpResponse.builder().statusCode(204)
.payload(payloadFromResource("/server_allowed_arguments.json")).build())
.putAll(requestsResponses).build(), new AbstractModule() {
@Override
protected void configure() {
bind(PasswordProvider.class).toInstance(passwordGenerator());
}
}).getContext();
.putAll(requestsResponses).build()).getContext();
}
protected ComputeServiceContext computeContextForKnownArgumentsAndConstantPassword() {

View File

@ -19,6 +19,8 @@
package org.jclouds.glesys.compute.options;
import static org.jclouds.glesys.compute.options.GleSYSTemplateOptions.Builder.ip;
import static org.jclouds.glesys.compute.options.GleSYSTemplateOptions.Builder.rootPassword;
import static org.jclouds.glesys.compute.options.GleSYSTemplateOptions.Builder.transferGB;
import static org.testng.Assert.assertEquals;
import org.jclouds.compute.options.TemplateOptions;
@ -57,8 +59,72 @@ public class GleSYSTemplateOptionsTest {
assertEquals(options.as(GleSYSTemplateOptions.class).getIp(), "1.1.1.1");
}
@Test(expectedExceptions = NullPointerException.class)
public void testNullIpThrowsNPE() {
new GleSYSTemplateOptions().ip(null);
}
@Test(expectedExceptions = IllegalArgumentException.class)
public void testInvalidIpThrowsIllegalArgument() {
new GleSYSTemplateOptions().ip("1.1.1");
}
@Test(expectedExceptions = IllegalArgumentException.class)
public void testipIsInvalidThrowsIllegalArgument() {
new GleSYSTemplateOptions().ip("foo");
}
@Test
public void testDefaultRootPassword() {
TemplateOptions options = new GleSYSTemplateOptions();
assertEquals(options.as(GleSYSTemplateOptions.class).getRootPassword(), null);
}
@Test
public void testRootPassword() {
TemplateOptions options = new GleSYSTemplateOptions().rootPassword("secret");
assertEquals(options.as(GleSYSTemplateOptions.class).getRootPassword(), "secret");
}
@Test
public void testRootPasswordStatic() {
TemplateOptions options = rootPassword("secret");
assertEquals(options.as(GleSYSTemplateOptions.class).getRootPassword(), "secret");
}
@Test(expectedExceptions = NullPointerException.class)
public void testNullRootPasswordThrowsNPE() {
new GleSYSTemplateOptions().rootPassword(null);
}
@Test
public void testDefaultTranferGB() {
TemplateOptions options = new GleSYSTemplateOptions();
assertEquals(options.as(GleSYSTemplateOptions.class).getTransferGB(), 50);
}
@Test
public void testTransferGB() {
TemplateOptions options = new GleSYSTemplateOptions().transferGB(75);
assertEquals(options.as(GleSYSTemplateOptions.class).getTransferGB(), 75);
}
@Test
public void testTransferGBStatic() {
TemplateOptions options = transferGB(75);
assertEquals(options.as(GleSYSTemplateOptions.class).getTransferGB(), 75);
}
@Test(expectedExceptions = IllegalArgumentException.class)
public void testNegativeTransferGBThrowsException() {
new GleSYSTemplateOptions().transferGB(-1);
}
@Test
public void testClone() {
GleSYSTemplateOptions clone = transferGB(75).rootPassword("root").ip("1.1.1.1").clone();
assertEquals(clone.getTransferGB(), 75);
assertEquals(clone.getRootPassword(), "root");
assertEquals(clone.getIp(), "1.1.1.1");
}
}