You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2015/09/09 00:10:06 UTC

[jclouds] Take into account that hardware profiles may not have scratch disks (#854)

The `GoogleComputeEngineServiceLiveTest#testListSizes` live test has started failing recently. I've observed that the API is no longer returning the [scratchDisks](https://github.com/jclouds/jclouds/blob/master/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/MachineType.java#L64) used to build the [hardware profile volumes](https://github.com/jclouds/jclouds/blob/master/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/MachineTypeToHardware.java#L72-L82), causing the test to fail [here](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java#L847).

Given that it is ok that in GCE the MachineType might not contain scratch disks, there are several ways to address this, including:

* Just override the test and remove the disks size assertion.
* If the MachineType has no scratch disks, add a *dummy* volume (note that its size is nullable), just to mark that at instance creation time a disk [will be created by default](https://github.com/jclouds/jclouds/blob/master/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java#L125) (the one implemented in this PR).
* Always add this *dummy* disk to represent the disk we create for the image.

@danbroudy how do you think this should be better fixed?
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/854

-- Commit Summary --

  * Take into account that hardware profiles may not have scratch disks

-- File Changes --

    M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/MachineTypeToHardware.java (19)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/854.patch
https://github.com/jclouds/jclouds/pull/854.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854

Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)

Posted by Ignasi Barrera <no...@github.com>.
Live tests are passing again. Pushed to master as [042222b6]( http://git-wip-us.apache.org/repos/asf/jclouds/commit/042222b6). Thanks @danbroudy!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854#issuecomment-141379905

Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)

Posted by Ignasi Barrera <no...@github.com>.
Closed #854.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854#event-412982094

Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)

Posted by Ignasi Barrera <no...@github.com>.
In most providers the volumes are just read only values to inform the user of what will be created when provisioning the VMs. In AWS-EC2 you have very detailed information on the type of the disks, etc, and users may want to pick a hardware profile or another based on that information.

I agree that returning a set of volumes with no information can be useless and confusing, and the purpose of remarking that a disk will be created is of little utility. After thinking a bit more abut this I think the best way to fix the tests is to accept the fact that many providers just don't have the notion of "volumes" at hardware profile level and consider that in the test base class, instead of overriding it.

Thanks for the feedback!


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854#issuecomment-140559710

Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)

Posted by Ignasi Barrera <no...@github.com>.
Just updated the PR to change the test instead of using dummy volumes.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854#issuecomment-140912431

Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)

Posted by danbroudy <no...@github.com>.
LGTM pending passing tests. Thanks for doing this!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854#issuecomment-141230817

Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)

Posted by danbroudy <no...@github.com>.
I would lean towards overriding the test and removing the disks size assertion. It seems potentially confusing to me that we would add a dummy volume. What are hardware profile volumes used for in practice? Why does a Jclouds user need these values?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/854#issuecomment-140286588