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/06/21 22:49:48 UTC
[jclouds] JCLOUDS-138. Add CloudStackImageExtension support. (#43)
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-1 jclouds-138
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/43
-- Commit Summary --
* JCLOUDS-138. Add CloudStackImageExtension support.
-- File Changes --
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java (12)
A apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackImageExtension.java (164)
A apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackImageExtensionExpectTest.java (200)
A apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackImageExtensionLiveTest.java (44)
A apis/cloudstack/src/test/resources/createtemplateresponse-imageextension.json (1)
A apis/cloudstack/src/test/resources/createtemplateresponse.json (1)
A apis/cloudstack/src/test/resources/listtemplatesresponse-imageextension.json (1)
A apis/cloudstack/src/test/resources/listvirtualmachinesresponse-imageextension.json (2)
A apis/cloudstack/src/test/resources/listvolumesreponse-imageextension.json (2)
A apis/cloudstack/src/test/resources/listvolumesresponse-imageextension.json (1)
A apis/cloudstack/src/test/resources/queryasyncjobresultresponse-createtemplate-imageextension.json (2)
A apis/cloudstack/src/test/resources/queryasyncjobresultresponse-stopvirtualmachine-imageextension.json (2)
A apis/cloudstack/src/test/resources/stopvirtualmachineresponse-imageextension.json (1)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/43.patch
https://github.com/jclouds/jclouds/pull/43.diff
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #471](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/471/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43#issuecomment-19928700
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Phillips <no...@github.com>.
> + String stopJob = client.getVirtualMachineClient().stopVirtualMachine(vm.getId());
> + boolean stopComplete = jobComplete.apply(stopJob);
> +
> + Set<Volume> volumes = client.getVolumeClient().listVolumes(ListVolumesOptions.Builder.virtualMachineId(vm.getId()));
> + Volume volume = Iterables.getOnlyElement(volumes);
> +
> + CreateTemplateOptions options = CreateTemplateOptions.Builder.volumeId(volume.getId());
> + AsyncCreateResponse templateJob = client.getTemplateClient().createTemplate(TemplateMetadata.builder()
> + .name(cloneTemplate.getName())
> + .osTypeId(vm.getGuestOSId())
> + .displayText(cloneTemplate.getName())
> + .build(), options);
> + Template newTemplate = blockUntilJobCompletesAndReturnResult.<Template>apply(templateJob);
> +
> + logger.info(">> Registered new template %s, waiting for it to become available.", newTemplate.getId());
> + System.out.println("locations: " + locations.get());
Is this a left-over debug statement? No `System.out.println` in code, I would assume...use `logger` instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4845999
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
> + String stopJob = client.getVirtualMachineClient().stopVirtualMachine(vm.getId());
> + boolean stopComplete = jobComplete.apply(stopJob);
> +
> + Set<Volume> volumes = client.getVolumeClient().listVolumes(ListVolumesOptions.Builder.virtualMachineId(vm.getId()));
> + Volume volume = Iterables.getOnlyElement(volumes);
> +
> + CreateTemplateOptions options = CreateTemplateOptions.Builder.volumeId(volume.getId());
> + AsyncCreateResponse templateJob = client.getTemplateClient().createTemplate(TemplateMetadata.builder()
> + .name(cloneTemplate.getName())
> + .osTypeId(vm.getGuestOSId())
> + .displayText(cloneTemplate.getName())
> + .build(), options);
> + Template newTemplate = blockUntilJobCompletesAndReturnResult.<Template>apply(templateJob);
> +
> + logger.info(">> Registered new template %s, waiting for it to become available.", newTemplate.getId());
> + System.out.println("locations: " + locations.get());
*whistles innocently* Why no, I didn't leave a debug statement there, what could you possibly be talking about? (delete delete delete...)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4848256
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #11](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/11/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43#issuecomment-19928725
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Phillips <no...@github.com>.
> + BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
> + Predicate<String> jobComplete) {
> + this.client = checkNotNull(client, "client");
> + this.userExecutor = checkNotNull(userExecutor, "userExecutor");
> + this.locations = checkNotNull(locations, "locations");
> + this.imageAvailablePredicate = checkNotNull(imageAvailablePredicate, "imageAvailablePredicate");
> + this.blockUntilJobCompletesAndReturnResult = checkNotNull(blockUntilJobCompletesAndReturnResult,
> + "blockUntilJobCompletesAndReturnResult");
> + this.jobComplete = checkNotNull(jobComplete, "jobComplete");
> + }
> +
> + @Override
> + public ImageTemplate buildImageTemplateFromNode(String name, final String id) {
> + VirtualMachine vm = client.getVirtualMachineClient().getVirtualMachine(id);
> + if (vm == null)
> + throw new NoSuchElementException("Cannot find vm with id: " + id);
Do we need `NoSuchElementException` here? If not, could `checkState` or `checkArgument` or so be used to shorten this, e.g.
```
VirtualMachine vm = checkNotNull(client.getVirtualMachineClient().getVirtualMachine(id), ...)
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4845958
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Phillips <no...@github.com>.
> + return image.get();
> + // TODO: get rid of the expectation that the image will be available, as it is very brittle
> + throw new UncheckedTimeoutException("Image was not created within the time limit: " + image.get());
> + }
> + });
> + }
> +
> + @Override
> + public boolean deleteImage(String id) {
> + try {
> + AsyncCreateResponse deleteJob = client.getTemplateClient().deleteTemplate(id);
> + boolean deleteComplete = jobComplete.apply(deleteJob.getJobId());
> + } catch (Exception e) {
> + return false;
> + }
> + return true;
Move into the `try` block?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4846021
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #4](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/4/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43#issuecomment-19842492
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
> + VirtualMachine vm = client.getVirtualMachineClient().getVirtualMachine(id);
> + if (vm == null)
> + throw new NoSuchElementException("Cannot find vm with id: " + id);
> + CloneImageTemplate template = new ImageTemplateBuilder.CloneImageTemplateBuilder().nodeId(id).name(name).build();
> + return template;
> + }
> +
> + @Override
> + public ListenableFuture<Image> createImage(ImageTemplate template) {
> + checkState(template instanceof CloneImageTemplate,
> + " cloudstack only currently supports creating images through cloning.");
> + CloneImageTemplate cloneTemplate = (CloneImageTemplate) template;
> +
> + VirtualMachine vm = client.getVirtualMachineClient().getVirtualMachine(cloneTemplate.getSourceNodeId());
> + String stopJob = client.getVirtualMachineClient().stopVirtualMachine(vm.getId());
> + boolean stopComplete = jobComplete.apply(stopJob);
At one point, I feel like I was using it, but now, no. Consider it gone.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4848240
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #464](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/464/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43#issuecomment-19842620
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
Closed #43.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
> + @Named(Constants.PROPERTY_USER_THREADS) ListeningExecutorService userExecutor,
> + @Memoized Supplier<Set<? extends Location>> locations,
> + @Named(TIMEOUT_IMAGE_AVAILABLE) Predicate<AtomicReference<Image>> imageAvailablePredicate,
> + BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
> + Predicate<String> jobComplete) {
> + this.client = checkNotNull(client, "client");
> + this.userExecutor = checkNotNull(userExecutor, "userExecutor");
> + this.locations = checkNotNull(locations, "locations");
> + this.imageAvailablePredicate = checkNotNull(imageAvailablePredicate, "imageAvailablePredicate");
> + this.blockUntilJobCompletesAndReturnResult = checkNotNull(blockUntilJobCompletesAndReturnResult,
> + "blockUntilJobCompletesAndReturnResult");
> + this.jobComplete = checkNotNull(jobComplete, "jobComplete");
> + }
> +
> + @Override
> + public ImageTemplate buildImageTemplateFromNode(String name, final String id) {
...not that I know of. Probably a remnant from the NovaImageExtension I started with. Fixing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4848201
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #135](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/135/) 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/43#issuecomment-19928171
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
> + BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
> + Predicate<String> jobComplete) {
> + this.client = checkNotNull(client, "client");
> + this.userExecutor = checkNotNull(userExecutor, "userExecutor");
> + this.locations = checkNotNull(locations, "locations");
> + this.imageAvailablePredicate = checkNotNull(imageAvailablePredicate, "imageAvailablePredicate");
> + this.blockUntilJobCompletesAndReturnResult = checkNotNull(blockUntilJobCompletesAndReturnResult,
> + "blockUntilJobCompletesAndReturnResult");
> + this.jobComplete = checkNotNull(jobComplete, "jobComplete");
> + }
> +
> + @Override
> + public ImageTemplate buildImageTemplateFromNode(String name, final String id) {
> + VirtualMachine vm = client.getVirtualMachineClient().getVirtualMachine(id);
> + if (vm == null)
> + throw new NoSuchElementException("Cannot find vm with id: " + id);
Consistency in behavior with EC2ImageExtension and NovaImageExtension - they both throw a NoSuchElementException as well, and I'd like to have the same behavior across the board.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4848229
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
fwiw, this is going to end up going into 1.6.x as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43#issuecomment-19841819
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
Committed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43#issuecomment-20077305
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Phillips <no...@github.com>.
> + VirtualMachine vm = client.getVirtualMachineClient().getVirtualMachine(id);
> + if (vm == null)
> + throw new NoSuchElementException("Cannot find vm with id: " + id);
> + CloneImageTemplate template = new ImageTemplateBuilder.CloneImageTemplateBuilder().nodeId(id).name(name).build();
> + return template;
> + }
> +
> + @Override
> + public ListenableFuture<Image> createImage(ImageTemplate template) {
> + checkState(template instanceof CloneImageTemplate,
> + " cloudstack only currently supports creating images through cloning.");
> + CloneImageTemplate cloneTemplate = (CloneImageTemplate) template;
> +
> + VirtualMachine vm = client.getVirtualMachineClient().getVirtualMachine(cloneTemplate.getSourceNodeId());
> + String stopJob = client.getVirtualMachineClient().stopVirtualMachine(vm.getId());
> + boolean stopComplete = jobComplete.apply(stopJob);
Is `stopComplete` used anywhere?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4845979
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Bayer <no...@github.com>.
> + return image.get();
> + // TODO: get rid of the expectation that the image will be available, as it is very brittle
> + throw new UncheckedTimeoutException("Image was not created within the time limit: " + image.get());
> + }
> + });
> + }
> +
> + @Override
> + public boolean deleteImage(String id) {
> + try {
> + AsyncCreateResponse deleteJob = client.getTemplateClient().deleteTemplate(id);
> + boolean deleteComplete = jobComplete.apply(deleteJob.getJobId());
> + } catch (Exception e) {
> + return false;
> + }
> + return true;
Okiedokie.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4848260
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #127](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/127/) 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/43#issuecomment-19842516
Re: [jclouds] JCLOUDS-138. Add CloudStackImageExtension support.
(#43)
Posted by Andrew Phillips <no...@github.com>.
> + @Named(Constants.PROPERTY_USER_THREADS) ListeningExecutorService userExecutor,
> + @Memoized Supplier<Set<? extends Location>> locations,
> + @Named(TIMEOUT_IMAGE_AVAILABLE) Predicate<AtomicReference<Image>> imageAvailablePredicate,
> + BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
> + Predicate<String> jobComplete) {
> + this.client = checkNotNull(client, "client");
> + this.userExecutor = checkNotNull(userExecutor, "userExecutor");
> + this.locations = checkNotNull(locations, "locations");
> + this.imageAvailablePredicate = checkNotNull(imageAvailablePredicate, "imageAvailablePredicate");
> + this.blockUntilJobCompletesAndReturnResult = checkNotNull(blockUntilJobCompletesAndReturnResult,
> + "blockUntilJobCompletesAndReturnResult");
> + this.jobComplete = checkNotNull(jobComplete, "jobComplete");
> + }
> +
> + @Override
> + public ImageTemplate buildImageTemplateFromNode(String name, final String id) {
Any reason `id` is `final` here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/43/files#r4845913