You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Bayer <no...@github.com> on 2013/08/12 21:52:49 UTC
[jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2 (#109)
Full implementation available for EC2, other ComputeServices updated
to not break due to not having VolumeExtensions.
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-1 jclouds-239
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/109
-- Commit Summary --
* JCLOUDS-239. Add VolumeExtension to compute and EC2
-- File Changes --
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/CloudStackComputeService.java (34)
M apis/ec2/pom.xml (4)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java (16)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceContextModule.java (6)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceDependenciesModule.java (13)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/extensions/EC2SecurityGroupExtension.java (2)
A apis/ec2/src/main/java/org/jclouds/ec2/compute/extensions/EC2VolumeExtension.java (466)
A apis/ec2/src/main/java/org/jclouds/ec2/compute/extensions/options/EC2DetachVolumeOptions.java (128)
A apis/ec2/src/main/java/org/jclouds/ec2/compute/extensions/options/EC2VolumeOptions.java (164)
A apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/EC2SnapshotToSnapshot.java (54)
A apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/EC2VolumeToVolume.java (87)
M apis/ec2/src/main/java/org/jclouds/ec2/domain/Volume.java (2)
M apis/ec2/src/main/java/org/jclouds/ec2/features/ElasticBlockStoreApi.java (14)
M apis/ec2/src/main/java/org/jclouds/ec2/predicates/SnapshotCompleted.java (2)
A apis/ec2/src/main/java/org/jclouds/ec2/predicates/VolumeDeleted.java (64)
M apis/ec2/src/test/java/org/jclouds/ec2/compute/extensions/EC2SecurityGroupExtensionExpectTest.java (4)
A apis/ec2/src/test/java/org/jclouds/ec2/compute/extensions/EC2VolumeExtensionExpectTest.java (517)
A apis/ec2/src/test/java/org/jclouds/ec2/compute/extensions/EC2VolumeExtensionLiveTest.java (70)
A apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/EC2SnapshotToSnapshotTest.java (50)
A apis/ec2/src/test/java/org/jclouds/ec2/compute/functions/EC2VolumeToVolumeTest.java (153)
A apis/ec2/src/test/resources/attach_volume_extension.xml (10)
A apis/ec2/src/test/resources/created_snapshot_extension.xml (10)
A apis/ec2/src/test/resources/created_volume_from_snapshot.xml (8)
A apis/ec2/src/test/resources/delete_snapshot.xml (4)
A apis/ec2/src/test/resources/delete_volume.xml (4)
A apis/ec2/src/test/resources/describe_instances_multiple_volume_extension.xml (130)
A apis/ec2/src/test/resources/describe_instances_running_volume_extension.xml (74)
A apis/ec2/src/test/resources/describe_snapshots_extension_created.xml (20)
A apis/ec2/src/test/resources/describe_snapshots_extension_deleted.xml (3)
A apis/ec2/src/test/resources/describe_volumes_extension.xml (32)
A apis/ec2/src/test/resources/describe_volumes_extension_deleted.xml (14)
A apis/ec2/src/test/resources/describe_volumes_extension_detached.xml (14)
A apis/ec2/src/test/resources/describe_volumes_extension_new_empty.xml (15)
A apis/ec2/src/test/resources/describe_volumes_extension_new_snapshot.xml (14)
A apis/ec2/src/test/resources/describe_volumes_extension_node.xml (23)
A apis/ec2/src/test/resources/detach_volume.xml (10)
A apis/ec2/src/test/resources/regionEndpoints-single-region.xml (10)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeService.java (13)
M common/trmk/src/main/java/org/jclouds/trmk/vcloud_0_8/compute/TerremarkVCloudComputeService.java (13)
M compute/src/main/java/org/jclouds/compute/ComputeService.java (10)
M compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java (7)
A compute/src/main/java/org/jclouds/compute/domain/Snapshot.java (73)
A compute/src/main/java/org/jclouds/compute/domain/SnapshotBuilder.java (82)
M compute/src/main/java/org/jclouds/compute/domain/Volume.java (25)
M compute/src/main/java/org/jclouds/compute/domain/VolumeBuilder.java (20)
A compute/src/main/java/org/jclouds/compute/domain/internal/SnapshotImpl.java (138)
M compute/src/main/java/org/jclouds/compute/domain/internal/VolumeImpl.java (41)
A compute/src/main/java/org/jclouds/compute/extensions/VolumeExtension.java (155)
A compute/src/main/java/org/jclouds/compute/extensions/options/AttachVolumeOptions.java (128)
A compute/src/main/java/org/jclouds/compute/extensions/options/CreateSnapshotOptions.java (128)
A compute/src/main/java/org/jclouds/compute/extensions/options/DetachVolumeOptions.java (160)
A compute/src/main/java/org/jclouds/compute/extensions/options/VolumeOptions.java (164)
M compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java (14)
M compute/src/main/java/org/jclouds/compute/predicates/ImagePredicates.java (6)
A compute/src/main/java/org/jclouds/compute/predicates/SnapshotPredicates.java (136)
A compute/src/main/java/org/jclouds/compute/predicates/VolumePredicates.java (179)
M compute/src/main/java/org/jclouds/compute/stub/config/StubComputeServiceContextModule.java (5)
M compute/src/main/java/org/jclouds/compute/stub/config/StubComputeServiceDependenciesModule.java (167)
A compute/src/main/java/org/jclouds/compute/stub/extensions/StubVolumeExtension.java (266)
A compute/src/test/java/org/jclouds/compute/extensions/internal/BaseVolumeExtensionLiveTest.java (296)
A compute/src/test/java/org/jclouds/compute/stub/extensions/StubVolumeExtensionIntegrationTest.java (52)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java (14)
M providers/gogrid/src/main/java/org/jclouds/gogrid/compute/GoGridComputeService.java (13)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/109.patch
https://github.com/jclouds/jclouds/pull/109.diff
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +
> + /**
> + * Remove a given volume.
> + *
> + * @param id The id of the volume to remove.
> + * @return true if the volume was successfully removed, false otherwise.
> + */
> + boolean removeVolume(String id);
> +
> + /**
> + * Attaches a volume to a node.
> + *
> + * @param volumeId The id of the volume to attach.
> + * @param nodeId The id of the node to attach the volume to.
> + * @param options Options for attaching the volume.
> + * @return True if we've successfully attached the volume, false otherwise.
'k - I've been doing booleans rather than exceptions with the SecurityGroupExtension too, following the model of deleteImage in ImageExtension, but I'm game for refactoring to do exceptions if that feels right.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468059
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #139](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/139/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-22524188
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +
> + /**
> + * List the snapshots in a given @{link Location}.
> + *
> + * @param locationId The location to search in.
> + * @return the set of @{link Snapshot}s we have access to in the given @{link Location}.
> + */
> + Set<Snapshot> listSnapshotsInLocation(String locationId);
> +
> + /**
> + * List the snapshots associated with a given volume.
> + *
> + * @param id The id of the volume to look at.
> + * @return the set of @{link Snapshot}s associated with this volume, if any.
> + */
> + Set<Snapshot> listSnapshotsForVolume(String id);
Yeah, the approach I was gonna go for was to squash down to a flat Set in the Extension implementations. But I defer unto consensus on the right way to go.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468084
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +
> + /**
> + * List the snapshots in a given @{link Location}.
> + *
> + * @param locationId The location to search in.
> + * @return the set of @{link Snapshot}s we have access to in the given @{link Location}.
> + */
> + Set<Snapshot> listSnapshotsInLocation(String locationId);
> +
> + /**
> + * List the snapshots associated with a given volume.
> + *
> + * @param id The id of the volume to look at.
> + * @return the set of @{link Snapshot}s associated with this volume, if any.
> + */
> + Set<Snapshot> listSnapshotsForVolume(String id);
Set is probably the wrong collection, as there could be tons.. Iterable or subtype (remember fluentiterable/multi-page in openstack)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467547
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +import org.jclouds.compute.extensions.options.CreateSnapshotOptions;
> +import org.jclouds.compute.extensions.options.DetachVolumeOptions;
> +import org.jclouds.compute.extensions.options.VolumeOptions;
> +
> +/**
> + * An extension to compute services to provide an abstraction for listing,
> + * creating, attaching, detaching and destroying volumes/disks.
> + *
> + * @author Andrew Bayer
> + */
> +public interface VolumeExtension {
> +
> + /**
> + * List volumes.
> + *
> + * @return The set of @{link Volume}s we have access to.
invalid javadoc
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467437
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> + * 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.compute.extensions.options;
> +
> +
> +import static com.google.common.base.Objects.equal;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Options supported by {@code VolumeExtension#createSnapshot}.
> + *
> + * APIs/providers implementing @{link VolumeExtension} will generally wish to
do a check on javadoc! `@{link` is invalid :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467590
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> * @return device this volume relates to on an operating system, if available
> */
> @Nullable
> String getDevice();
>
> /**
> - *
> + *
> + * @return the name of the volume, if available.
> + */
> + @Nullable
> + String getName();
> +
> + /**
> + * @return the location for this volume, if available
presumably `{@link Location#getId id}` right?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467410
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #231](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/231/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-24618325
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> + /**
> + * Detaches a volume from a node
> + *
> + * @param id The id of the volume to detach.
> + * @param options Options for detaching the volume.
> + *
> + * @return true if we've successfully detached the volume, false otherwise.
> + */
> + boolean detachVolume(String id, DetachVolumeOptions options);
> +
> + /**
> + * List snapshots.
> + *
> + * @return The set of @{link Snapshot}s we have access to.
> + */
> + Set<Snapshot> listSnapshots();
weird there's a name field but no query by name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467614
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.compute.extensions.options;
> +
> +
> +import static com.google.common.base.Objects.equal;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Options supported by {@code VolumeExtension#detachVolume}.
> + *
> + * APIs/providers implementing @{link VolumeExtension} will generally wish to
> + * extend DetachVolumeOptions, as is done with @{link TemplateOptions}
> + */
> +public class DetachVolumeOptions implements Cloneable {
Oh, turns out GCE needs device as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6469460
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
would be nice to summarize the design in the description of this PR. Ex. what the scope is of the abstraction (readonly vs readwrite, constraints, code example for each).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-24758989
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +import org.jclouds.compute.extensions.options.VolumeOptions;
> +
> +/**
> + * An extension to compute services to provide an abstraction for listing,
> + * creating, attaching, detaching and destroying volumes/disks.
> + *
> + * @author Andrew Bayer
> + */
> +public interface VolumeExtension {
> +
> + /**
> + * List volumes.
> + *
> + * @return The set of @{link Volume}s we have access to.
> + */
> + Set<Volume> listVolumes();
Okiedokie.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467865
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +
> + /**
> + * Remove a given volume.
> + *
> + * @param id The id of the volume to remove.
> + * @return true if the volume was successfully removed, false otherwise.
> + */
> + boolean removeVolume(String id);
> +
> + /**
> + * Attaches a volume to a node.
> + *
> + * @param volumeId The id of the volume to attach.
> + * @param nodeId The id of the node to attach the volume to.
> + * @param options Options for attaching the volume.
> + * @return True if we've successfully attached the volume, false otherwise.
does this boolean value hold weight across clouds?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467500
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.compute.extensions.options;
> +
> +
> +import static com.google.common.base.Objects.equal;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Options supported by {@code VolumeExtension#detachVolume}.
> + *
> + * APIs/providers implementing @{link VolumeExtension} will generally wish to
> + * extend DetachVolumeOptions, as is done with @{link TemplateOptions}
> + */
> +public class DetachVolumeOptions implements Cloneable {
CloudStack doesn't require the instance ID for detaching, EC2 and Nova do. CloudStack and Nova don't do anything with device for detaching, but EC2 apparently does allow attaching to multiple devices, 'cos it does take the device as a parameter for detaching.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468343
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +
> + /**
> + * List the snapshots in a given @{link Location}.
> + *
> + * @param locationId The location to search in.
> + * @return the set of @{link Snapshot}s we have access to in the given @{link Location}.
> + */
> + Set<Snapshot> listSnapshotsInLocation(String locationId);
> +
> + /**
> + * List the snapshots associated with a given volume.
> + *
> + * @param id The id of the volume to look at.
> + * @return the set of @{link Snapshot}s associated with this volume, if any.
> + */
> + Set<Snapshot> listSnapshotsForVolume(String id);
Well, we use set when set adds value. i recall a discussion w tperiels
about this some time back.
On Thu, Sep 19, 2013 at 8:10 PM, Andrew Bayer <no...@github.com>wrote:
> In
> compute/src/main/java/org/jclouds/compute/extensions/VolumeExtension.java:
>
> > +
> > + /**
> > + * List the snapshots in a given @{link Location}.
> > + *
> > + * @param locationId The location to search in.
> > + * @return the set of @{link Snapshot}s we have access to in the given @{link Location}.
> > + */
> > + Set<Snapshot> listSnapshotsInLocation(String locationId);
> > +
> > + /**
> > + * List the snapshots associated with a given volume.
> > + *
> > + * @param id The id of the volume to look at.
> > + * @return the set of @{link Snapshot}s associated with this volume, if any.
> > + */
> > + Set<Snapshot> listSnapshotsForVolume(String id);
>
> Yeah, the approach I was gonna go for was to squash down to a flat Set in
> the Extension implementations. But I defer unto consensus on the right way
> to go.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/109/files#r6468084>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6469078
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> * @return device this volume relates to on an operating system, if available
> */
> @Nullable
> String getDevice();
>
> /**
> - *
> + *
> + * @return the name of the volume, if available.
will do - it's for being able to search by volume name when appropriate.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467838
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.compute.extensions.options;
> +
> +
> +import static com.google.common.base.Objects.equal;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Options supported by {@code VolumeExtension#detachVolume}.
> + *
> + * APIs/providers implementing @{link VolumeExtension} will generally wish to
> + * extend DetachVolumeOptions, as is done with @{link TemplateOptions}
> + */
> +public class DetachVolumeOptions implements Cloneable {
strange.. is instanceid really optional? also why is deviceId important? Is it possible to use the api to attach a volume to multiple devices?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467656
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> * @return device this volume relates to on an operating system, if available
> */
> @Nullable
> String getDevice();
>
> /**
> - *
> + *
> + * @return the name of the volume, if available.
> + */
> + @Nullable
> + String getName();
> +
> + /**
> + * @return the location for this volume, if available
will do.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467798
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> + }
> +
> + @Override
> + public String toString() {
> + return delegate.toString();
> + }
> +
> + public ImmutableAttachVolumeOptions(AttachVolumeOptions delegate) {
> + this.delegate = delegate;
> + }
> + }
> +
> + protected String device;
> +
> + /**
> + * @return The device to attach this volume to.
example.. (presumably raw device?)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467565
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> + /**
> + * Detaches a volume from a node
> + *
> + * @param id The id of the volume to detach.
> + * @param options Options for detaching the volume.
> + *
> + * @return true if we've successfully detached the volume, false otherwise.
> + */
> + boolean detachVolume(String id, DetachVolumeOptions options);
> +
> + /**
> + * List snapshots.
> + *
> + * @return The set of @{link Snapshot}s we have access to.
> + */
> + Set<Snapshot> listSnapshots();
Good point. Lemme try to remember what my reasoning was. I think it may be that some of the APIs require a name when creating a volume/snapshot, but not sure...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468171
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> <test.ec2.build-version />
> <test.ec2.identity>${test.aws.identity}</test.ec2.identity>
> <test.ec2.credential>${test.aws.credential}</test.ec2.credential>
> - <test.ec2.template />
> + <test.ec2.template>hardwareId=m1.small,imageId=us-east-1/ami-1ab3ce73</test.ec2.template>
comment about what this is an ami of
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467338
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +import org.jclouds.compute.extensions.options.VolumeOptions;
> +
> +/**
> + * An extension to compute services to provide an abstraction for listing,
> + * creating, attaching, detaching and destroying volumes/disks.
> + *
> + * @author Andrew Bayer
> + */
> +public interface VolumeExtension {
> +
> + /**
> + * List volumes.
> + *
> + * @return The set of @{link Volume}s we have access to.
> + */
> + Set<Volume> listVolumes();
nit: prefer leaving off Volume from method names in the Volume extension
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467434
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Everett Toews <no...@github.com>.
During release week we like to do a little house cleaning in the jclouds world. That means sweeping out the pull request queue.
This PR is over 6 months old. Please update us on its status here. If we don't hear anything, we will take that as lazy consensus that the PR is no longer relevant and it will be closed on Friday, May 30.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-44344966
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #597](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/597/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-22523772
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> + * 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.compute.extensions.options;
> +
> +
> +import static com.google.common.base.Objects.equal;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Options supported by {@code VolumeExtension#createSnapshot}.
> + *
> + * APIs/providers implementing @{link VolumeExtension} will generally wish to
fix'd!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468142
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #199](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/199/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-23832483
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +
> + /**
> + * List the volumes in a given @{link Location}.
> + *
> + * @param locationId The location to search in.
> + * @return the set of @{link Volume}s we have access to in the given @{link Location}.
> + */
> + Set<Volume> listVolumesInLocation(String locationId);
> +
> + /**
> + * List the volumes associated with a given node.
> + *
> + * @param id The id of the node to look at.
> + * @return the set of @{link Volume}s associated with this node, if any.
> + */
> + Set<Volume> listVolumesForNode(String id);
volumes attached to node? or is there another relationship..
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467482
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +import org.jclouds.compute.extensions.options.CreateSnapshotOptions;
> +import org.jclouds.compute.extensions.options.DetachVolumeOptions;
> +import org.jclouds.compute.extensions.options.VolumeOptions;
> +
> +/**
> + * An extension to compute services to provide an abstraction for listing,
> + * creating, attaching, detaching and destroying volumes/disks.
> + *
> + * @author Andrew Bayer
> + */
> +public interface VolumeExtension {
> +
> + /**
> + * List volumes.
> + *
> + * @return The set of @{link Volume}s we have access to.
in fact, remove javadoc like this unless it says something notable..
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467452
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #692](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/692/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-24616834
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +
> + /**
> + * Remove a given volume.
> + *
> + * @param id The id of the volume to remove.
> + * @return true if the volume was successfully removed, false otherwise.
> + */
> + boolean removeVolume(String id);
> +
> + /**
> + * Attaches a volume to a node.
> + *
> + * @param volumeId The id of the volume to attach.
> + * @param nodeId The id of the node to attach the volume to.
> + * @param options Options for attaching the volume.
> + * @return True if we've successfully attached the volume, false otherwise.
prefer to throw an exception, as I suspect it wont
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467513
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> + }
> +
> + @Override
> + public String toString() {
> + return delegate.toString();
> + }
> +
> + public ImmutableAttachVolumeOptions(AttachVolumeOptions delegate) {
> + this.delegate = delegate;
> + }
> + }
> +
> + protected String device;
> +
> + /**
> + * @return The device to attach this volume to.
Yup, clarified in javadoc.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468368
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Everett Toews <no...@github.com>.
Closed #109.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#event-126633616
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
done reviewing model, will switch to impl after model comments come back
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-24759926
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #379](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/379/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-23832198
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #308](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/308/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-22523568
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +import com.google.inject.ImplementedBy;
> +
> +/**
> + * Describes a snapshot of a {@link Volume}
> + *
> + * @author Andrew Bayer
> + */
> +@ImplementedBy(SnapshotImpl.class)
> +public interface Snapshot {
> +
> + /**
> + * Unique identifier.
> + *
> + */
> + @Nullable
> + String getId();
hard to believe everything is nullable.. can you take a pass at this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467361
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> * @return device this volume relates to on an operating system, if available
> */
> @Nullable
> String getDevice();
>
> /**
> - *
> + *
> + * @return the name of the volume, if available.
maybe some text about what this is used for?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467416
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +import com.google.inject.ImplementedBy;
> +
> +/**
> + * Describes a snapshot of a {@link Volume}
> + *
> + * @author Andrew Bayer
> + */
> +@ImplementedBy(SnapshotImpl.class)
> +public interface Snapshot {
> +
> + /**
> + * Unique identifier.
> + *
> + */
> + @Nullable
> + String getId();
re: nullable -yeah, I was overly cautious there. Fixing!
re: interface/impl - I was just going with how all the other things in compute.domain seem to be - I'm fine either way.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467543
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +import com.google.inject.ImplementedBy;
> +
> +/**
> + * Describes a snapshot of a {@link Volume}
> + *
> + * @author Andrew Bayer
> + */
> +@ImplementedBy(SnapshotImpl.class)
> +public interface Snapshot {
> +
> + /**
> + * Unique identifier.
> + *
> + */
> + @Nullable
> + String getId();
looks like id, volume id, and date created can all be non-nullable, but the others aren't consistent across all of CloudStack/EC2/Nova, so I'll leave them nullable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467761
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.compute.extensions.options;
> +
> +
> +import static com.google.common.base.Objects.equal;
> +
> +import com.google.common.base.Objects;
> +
> +/**
> + * Options supported by {@code VolumeExtension#detachVolume}.
> + *
> + * APIs/providers implementing @{link VolumeExtension} will generally wish to
> + * extend DetachVolumeOptions, as is done with @{link TemplateOptions}
> + */
> +public class DetachVolumeOptions implements Cloneable {
so I'd remove the device field from this and punt to a list of devices in
an EC2 subtype
On Thu, Sep 19, 2013 at 8:17 PM, Andrew Bayer <no...@github.com>wrote:
> In
> compute/src/main/java/org/jclouds/compute/extensions/options/DetachVolumeOptions.java:
>
> > + * limitations under the License.
> > + */
> > +package org.jclouds.compute.extensions.options;
> > +
> > +
> > +import static com.google.common.base.Objects.equal;
> > +
> > +import com.google.common.base.
> Objects;
> > +
> > +/**
> > + * Options supported by {@code VolumeExtension#detachVolume}.
> > + *
> > + * APIs/providers implementing @{link VolumeExtension} will generally wish to
> > + * extend DetachVolumeOptions, as is done with @{link TemplateOptions}
> > + */
> > +public class DetachVolumeOptions implements Cloneable {
>
> CloudStack doesn't require the instance ID for detaching, EC2 and Nova do.
> CloudStack and Nova don't do anything with device for detaching, but EC2
> apparently does allow attaching to multiple devices, 'cos it does take the
> device as a parameter for detaching.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/109/files#r6468343>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6469124
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Adrian Cole <no...@github.com>.
> +import com.google.inject.ImplementedBy;
> +
> +/**
> + * Describes a snapshot of a {@link Volume}
> + *
> + * @author Andrew Bayer
> + */
> +@ImplementedBy(SnapshotImpl.class)
> +public interface Snapshot {
> +
> + /**
> + * Unique identifier.
> + *
> + */
> + @Nullable
> + String getId();
is it helpful to separate interface from impl in this class? i.e. is there a reasonable chance of useful alternate impls?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6467386
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +import org.jclouds.compute.extensions.options.CreateSnapshotOptions;
> +import org.jclouds.compute.extensions.options.DetachVolumeOptions;
> +import org.jclouds.compute.extensions.options.VolumeOptions;
> +
> +/**
> + * An extension to compute services to provide an abstraction for listing,
> + * creating, attaching, detaching and destroying volumes/disks.
> + *
> + * @author Andrew Bayer
> + */
> +public interface VolumeExtension {
> +
> + /**
> + * List volumes.
> + *
> + * @return The set of @{link Volume}s we have access to.
I dunno, some boilerplate javadoc seems to have a bit of value...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468007
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +
> + /**
> + * List the volumes in a given @{link Location}.
> + *
> + * @param locationId The location to search in.
> + * @return the set of @{link Volume}s we have access to in the given @{link Location}.
> + */
> + Set<Volume> listVolumesInLocation(String locationId);
> +
> + /**
> + * List the volumes associated with a given node.
> + *
> + * @param id The id of the node to look at.
> + * @return the set of @{link Volume}s associated with this node, if any.
> + */
> + Set<Volume> listVolumesForNode(String id);
Yeah, you're right. Clarifying.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6468015
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #660](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/660/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109#issuecomment-23832639
Re: [jclouds] JCLOUDS-239. Add VolumeExtension to compute and EC2
(#109)
Posted by Andrew Bayer <no...@github.com>.
> +
> + /**
> + * List the snapshots in a given @{link Location}.
> + *
> + * @param locationId The location to search in.
> + * @return the set of @{link Snapshot}s we have access to in the given @{link Location}.
> + */
> + Set<Snapshot> listSnapshotsInLocation(String locationId);
> +
> + /**
> + * List the snapshots associated with a given volume.
> + *
> + * @param id The id of the volume to look at.
> + * @return the set of @{link Snapshot}s associated with this volume, if any.
> + */
> + Set<Snapshot> listSnapshotsForVolume(String id);
Alright. Iterable, then, I guess?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/109/files#r6469627