You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Bhathiya <no...@github.com> on 2015/04/14 14:33:10 UTC
[jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Virtual Machine Image Operations.
https://msdn.microsoft.com/en-us/library/azure/dn499771.aspx
Rebased
https://github.com/jclouds/jclouds-labs/pull/144 on master
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/167
-- Commit Summary --
* JCLOUDS-852:Added VMImageOpertaions
-- File Changes --
M azurecompute/src/main/java/org/jclouds/azurecompute/AzureComputeApi.java (6)
A azurecompute/src/main/java/org/jclouds/azurecompute/binders/CaptureVMImageParamsToXML.java (68)
A azurecompute/src/main/java/org/jclouds/azurecompute/binders/VMImageParamsToXML.java (94)
A azurecompute/src/main/java/org/jclouds/azurecompute/domain/CaptureVMImageParams.java (117)
A azurecompute/src/main/java/org/jclouds/azurecompute/domain/VMImage.java (169)
A azurecompute/src/main/java/org/jclouds/azurecompute/domain/VMImageParams.java (247)
A azurecompute/src/main/java/org/jclouds/azurecompute/features/VMImageApi.java (98)
M azurecompute/src/main/java/org/jclouds/azurecompute/features/VirtualMachineApi.java (13)
M azurecompute/src/main/java/org/jclouds/azurecompute/xml/DataVirtualHardDiskHandler.java (2)
A azurecompute/src/main/java/org/jclouds/azurecompute/xml/ListVMImagesHandler.java (70)
A azurecompute/src/main/java/org/jclouds/azurecompute/xml/OSConfigHandler.java (105)
A azurecompute/src/main/java/org/jclouds/azurecompute/xml/VMImageHandler.java (207)
A azurecompute/src/test/java/org/jclouds/azurecompute/features/VMImageApiLiveTest.java (205)
A azurecompute/src/test/java/org/jclouds/azurecompute/features/VMImageApiMockTest.java (103)
A azurecompute/src/test/java/org/jclouds/azurecompute/xml/ListVMImagesHandlerTest.java (106)
A azurecompute/src/test/resources/vmimageparams.xml (1)
A azurecompute/src/test/resources/vmimages.xml (56)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/167.patch
https://github.com/jclouds/jclouds-labs/pull/167.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
> +
> + private VMImage.OSDiskConfiguration.Caching hostCaching;
> + private String name;
> + private VMImage.OSDiskConfiguration.OSState osState;
> + private OSImage.Type os;
> + private Integer logicalDiskSizeInGB;
> + private URI mediaLink;
> + private String ioType;
> +
> + private final StringBuilder currentText = new StringBuilder();
> +
> + @Override
> + public VMImage.OSDiskConfiguration getResult() {
> + VMImage.OSDiskConfiguration result = VMImage.OSDiskConfiguration
> + .create(name, hostCaching, osState, os, mediaLink, logicalDiskSizeInGB, ioType);
> + return result;
can we just return it directly?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28420287
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
> + VMImageParams.OSDiskConfigurationParams.Caching.READ_ONLY,
> + VMImageParams.OSDiskConfigurationParams.OSState.SPECIALIZED,
> + image.osDiskConfiguration().os(),
> + URI.create(
> + "https://" + storageService.serviceName()
> + + ".blob.core.windows.net/vhds/" + CAPTURED_IMAGE_NAME + "-os-" + dateFormat.format(date) + ".vhd"),
> + 30,
> + "Standard");
> + VMImageParams params = VMImageParams.builder().name(CREATE_IMAGE_NAME).label(CREATE_IMAGE_NAME)
> + .description(image.description()).recommendedVMSize(image.recommendedVMSize())
> + .osDiskConfiguration(osParams).imageFamily(image.imageFamily())
> + .build();
> +
> + String requestId = api().create(params);
> + assertNotNull(requestId);
> + operationSucceeded.apply(requestId);
Is this something that needs to be deleted at some point when the test finishes?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28432763
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
Amended de commit message and pushed to [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/882be03e) and [1.9.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/6b4de9e7). Thanks @hsbhathiya!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-97832937
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> + @Named("UpdateVMImage")
> + @PUT
> + @Path("/{imageName}")
> + @Produces(APPLICATION_XML)
> + @ResponseParser(ParseRequestIdHeader.class) String update(String imageName,
> + @BinderParam(VMImageParamsToXML.class) VMImageParams params);
> +
> + /**
> + * The Delete VM Image operation deletes the specified VM Image from the image repository that is associated with
> + * the specified subscription.
> + */
> + @Named("DeleteImage")
> + @DELETE
> + @Path("/{imageName}")
> + @Produces(APPLICATION_XML)
Same comment about the produced body and the annotation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28432527
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
> + }
> +
> + public Builder fromVMImageParams(VMImageParams in) {
> + return name(in.name()).label(in.label()).description(in.description())
> + .osDiskConfiguration(in.osDiskConfiguration()).dataDiskConfigurations(in.dataDiskConfiguration())
> + .language(in.language()).imageFamily(in.imageFamily()).recommendedVMSize(in.recommendedVMSize())
> + .eula(in.eula()).iconUri(in.iconUri()).smallIconUri(in.smallIconUri()).privacyUri(in.privacyUri());
> + }
> +
> + }
> +
> + public static VMImageParams create(String name, String label, String description,
> + OSDiskConfigurationParams osDiskConfiguration, List<DataVirtualHardDisk> dataDiskConfiguration,
> + String language, String imageFamily, RoleSize.Type recommendedVMSize, String eula, URI iconUri,
> + URI smallIconUri, URI privacyUri, Boolean showGui) {
> + return new AutoValue_VMImageParams(name, label, description, osDiskConfiguration, dataDiskConfiguration,
for lists (or any other collections) in AutoValue classe, please use ImmutableList.copyOf(dataDiskConfiguration) as recommended by auto/value doc
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28420171
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see <a href="https://msdn.microsoft.com/en-us/library/azure/dn499771.aspx">docs</a>
> + */
> +@Path("/services/vmimages")
> +@Consumes(APPLICATION_XML)
> +@Headers(keys = "x-ms-version", values = "{jclouds.api-version}")
> +public interface VMImageApi {
> +
> +
> + /**
> + * The List VM Images operation retrieves a list of the VM Images from the image repository that is associated with
> + * the specified subscription.
> + */
> + @Named("ListVMImages")
> + @GET
> + @XMLResponseParser(ListVMImagesHandler.class)
> + @Produces(APPLICATION_XML)
Does this method generate a body?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28673779
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @hsbhathiya! lgtm. Mind squashing the commits?
Also, while squashing, mind adding a comment in the VirtualMachineApi mock and live test classes indicating that the capture tests are in the VMImage test classes? One would expect those to be in the VirtualMachineApi test classes, so let's make sure people will find those tests and we know they're not missing :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-95595319
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
> + .language("en")
> + .eula("http://www.gnu.org/copyleft/gpl.html")
> + .iconUri(URI.create("http://www.cloudera.com/content/cloudera/en/privacy-policy.html"))
> + .smallIconUri(URI.create("http://www.cloudera.com/content/cloudera/en/privacy-policy.html"))
> + .privacyUri(URI.create("http://www.cloudera.com/content/cloudera/en/privacy-policy.html"))
> + .showGui(Boolean.TRUE)
> + .build();
> +
> + assertEquals(api.create(params), "request-1");
> +
> + assertSent(server, "POST", "/services/vmimages", "/vmimageparams.xml");
> + } finally {
> + server.shutdown();
> + }
> + }
> +}
Add the missing mock tests: update, delete, and delete with fallback.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28432896
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
> +
> + @Nullable public abstract URI iconUri();
> +
> + @Nullable public abstract URI smallIconUri();
> +
> + @Nullable public abstract URI privacyUri();
> +
> + @Nullable public abstract Date publishedDate();
> +
> + public static VMImage create(String name, String label, String category, String description,
> + OSDiskConfiguration osDiskConfiguration, List<DataVirtualHardDisk> dataDiskConfiguration, String serviceName,
> + String deploymentName, String roleName, String location, String affinityGroup, Date createdTime,
> + Date modifiedTime, String language, String imageFamily, RoleSize.Type recommendedVMSize, Boolean isPremium,
> + String eula, URI iconUri, URI smallIconUri,
> + URI privacyUri, Date publishedDate) {
> + return new AutoValue_VMImage(name, label, category, description, osDiskConfiguration, dataDiskConfiguration,
use `ImmutableList.copyOf(dataDiskConfiguration)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28420235
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Francesco Chicchiriccò <no...@github.com>.
```
Tests run: 98, Failures: 0, Errors: 0, Skipped: 2
[...]
[INFO] Total time: 01:24 h
```
Looks good! @nacx @andreaturli could you please review this PR?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-93259276
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
Also add the mock and live tests for the method you added to the `VirtualMachineApi`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-93456947
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
@hsbhathiya I think it looks good, only minor comments from my side.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-93453856
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Bhathiya <no...@github.com>.
Ok I won't squash by now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-94408687
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
> +@AutoValue
> +public abstract class VMImage {
> +
> + @AutoValue
> + public abstract static class OSDiskConfiguration {
> +
> + public enum Caching {
> + READ_ONLY,
> + READ_WRITE,
> + NONE
> + }
> +
> + public enum OSState {
> + GENERALIZED,
> + SPECIALIZED
> +
please remove trailing lines
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28419969
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Bhathiya <no...@github.com>.
@nacx I have addressed issues.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-94900399
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
> + public abstract String name();
> +
> + /**
> + * Specifies the caching behavior of the operating system disk.
> + * This setting impacts the consistency and performance of the disk.
> + * The default value is ReadWrite
> + */
> + @Nullable public abstract Caching hostCaching();
> +
> + /**
> + * Specifies the state of the operating system in the image.
> + * A Virtual Machine that is fully configured and running contains a Specialized operating system.
> + * A Virtual Machine on which the Sysprep command has been run with the generalize option contains
> + * a Generalized operating system.
> + */
> +
remove line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28420032
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see <a href="https://msdn.microsoft.com/en-us/library/azure/dn499771.aspx">docs</a>
> + */
> +@Path("/services/vmimages")
> +@Consumes(APPLICATION_XML)
> +@Headers(keys = "x-ms-version", values = "{jclouds.api-version}")
> +public interface VMImageApi {
> +
> +
> + /**
> + * The List VM Images operation retrieves a list of the VM Images from the image repository that is associated with
> + * the specified subscription.
> + */
> + @Named("ListVMImages")
> + @GET
> + @XMLResponseParser(ListVMImagesHandler.class)
> + @Produces(APPLICATION_XML)
If this method does not produce a body, remove this annotation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28432458
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Ignasi Barrera <no...@github.com>.
>Also add the mock and live tests for the method you added to the VirtualMachineApi.
This still has to be done.
Please, when addressing review commits add the changes in separate commits. Don't squash them unless requested. That makes the review much more easy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-94402545
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Andrea Turli <no...@github.com>.
> + public abstract String name();
> +
> + /**
> + * Specifies the caching behavior of the operating system disk.
> + * This setting impacts the consistency and performance of the disk.
> + * The default value is ReadWrite
> + */
> + @Nullable public abstract Caching hostCaching();
> +
> + /**
> + * Specifies the state of the operating system in the image.
> + * A Virtual Machine that is fully configured and running contains a Specialized operating system.
> + * A Virtual Machine on which the Sysprep command has been run with the generalize option contains
> + * a Generalized operating system.
> + */
> +
remove this line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167/files#r28419997
Re: [jclouds-labs] JCLOUDS-852:Added VMImageOpertaions (#167)
Posted by Bhathiya <no...@github.com>.
@nacx
CaptureVMImage operation is added to the VirtualMachineApi becasuse of its Request URI. All the other VMImage Operations request URIs start with https://management.core.windows.net/<subscription-id>/services/vmimages. Tests for that method is in VMImgeLiveTests.
I will add the CaptureVMImage Mock test as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/167#issuecomment-94408140