devcsrj requested changes on this pull request.
Also, could you post a copy of your the build results of this module in a gist,
or a pastie?
P.S.: Take care, and make sure **not** to include your credentials!
> + TemplateOptions options = template.getOptions();
+ final String loginUser = isNullOrEmpty(options.getLoginUser()) ? "root"
: options.getLoginUser();
+ final String password = options.hasLoginPassword() ?
options.getLoginPassword() : Passwords.generate();
+ final org.jclouds.compute.domain.Image image = template.getImage();
+
+ // provision all volumes based on hardware
+ List<? extends Volume> volumes = hardware.getVolumes();
+ List<String> volumeIds =
Lists.newArrayListWithExpectedSize(volumes.size());
+
+ int i = 1;
+ for (final Volume volume : volumes) {
+ try {
+ logger.trace("<< provisioning volume '%s'", volume);
+ final
org.apache.jclouds.profitbricks.rest.domain.Volume.Request.CreatePayload
request =
org.apache.jclouds.profitbricks.rest.domain.Volume.Request.creatingBuilder()
+ .dataCenterId(dataCenterId)
+ .image(image.getId())
Is it intentional that here we provision every volume with an image? (i.e.: If
we attached 5 volumes, each has a bootable OS?)
> + logger.trace(">> provisioning complete for server. returned
> id='%s'", serverId);
+
+ } catch (Exception ex) {
+ logger.error(ex, ">> failed to provision server. rollbacking..");
+ destroyVolumes(volumeIds, dataCenterId);
+ throw Throwables.propagate(ex);
+ }
+
+ waitServerUntilAvailable.apply(ServerRef.create(dataCenterId, serverId));
+ waitDcUntilAvailable.apply(dataCenterId);
+
+ //attach bootVolume to Server
+ api.serverApi().attachVolume(Server.Request.attachVolumeBuilder()
+ .dataCenterId(dataCenterId)
+ .serverId(serverId)
+ .volumeId(bootVolume.id())
IIRC, if one creates a server (line 193) with the `bootVolume` param set, it'll
be automatically attached after that server is provisioned. Is this not the
case anymore?
> + public List<Snapshot> call() throws Exception {
+ logger.trace("<< fetching snapshots");
+ List<Snapshot> remoteSnapshots = api.snapshotApi().list(new
DepthOptions().depth(1));
+ logger.trace(">> snapshots feched.");
+
+ return remoteSnapshots;
+ }
+
+ });
+
+ return Iterables.concat(getUnchecked(images), getUnchecked(snapshots));
+ }
+
+ @Override
+ public Provisionable getImage(String id
+ ) {
Woops right paren
> DE_FKB("de/fkb", "Germany, Karlsruhe"),
DE_FRA("de/fra", "Germany, Frankfurt (M)"),
US_LAS("us/las", "USA, Las Vegas"),
US_LASDEV("us/lasdev", "USA Developer cluster"),
- UNRECOGNIZED("unrecognized", "Unrecognized location");
+ UNRECOGNIZED("unrecognized", "Unrecognized location"),
+ Mock("mock", "mock");
[nit] All caps
> return UNRECOGNIZED;
}
+
+ @Override
+ public String toString() {
+ return id;
Make this more descriptive, something like:
```
return "Location[id=" + id + ", description=" + description + "]";
```
..or don't override it at all, and use the actual `getId()` method, to print
out the ID.
> @@ -61,9 +66,9 @@
<scope>provided</scope>
</dependency>
<dependency>
- <groupId>org.apache.jclouds.driver</groupId>
- <artifactId>jclouds-okhttp</artifactId>
- <version>${jclouds.version}</version>
+ <groupId>org.apache.jclouds.driver</groupId>
+ <artifactId>jclouds-okhttp</artifactId>
+ <version>${jclouds.version}</version>
[nit] Formatting
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/292#pullrequestreview-884467
