You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Iván Lomba <no...@github.com> on 2016/08/15 14:52:02 UTC
[jclouds/jclouds] JCLOUDS-482: Add disk to AutomaticHardwareIdSpec
and support ProfitBricks (#997)
* Support disk on AutomatiHarwareIdSpec
* Add support for ProfitBricks
* Fix two conditions to avoid negative number of cores.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/997
-- Commit Summary --
* Fix assert on BaseTemplateBuilderLiveTest
* Fix cores condition on automaticHardwareIdSpecBuilder
* Minor fix: group name consistent with other tests
* Fix condition on resolveHardware from ArbitraryCpuRamTemplateBuilderImpl
* Add disk support on hardwareIdSpec
* Add support for ProfitBricks
-- File Changes --
M compute/src/main/java/org/jclouds/compute/domain/internal/ArbitraryCpuRamTemplateBuilderImpl.java (19)
M compute/src/main/java/org/jclouds/compute/util/AutomaticHardwareIdSpec.java (33)
M compute/src/test/java/org/jclouds/compute/domain/internal/ArbitraryCpuRamTemplateBuilderImplTest.java (100)
M compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java (4)
M compute/src/test/java/org/jclouds/compute/internal/BaseTemplateBuilderLiveTest.java (2)
M compute/src/test/java/org/jclouds/compute/util/AutomaticHardwareIdSpecTest.java (43)
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/domain/internal/GoogleComputeEngineArbitraryCpuRamTemplateBuilderImpl.java (3)
M providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceLiveTest.java (4)
M providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/domain/internal/GoogleComputeEngineArbitraryCpuRamTemplateBuilderImplTest.java (24)
M providers/profitbricks/src/main/java/org/jclouds/profitbricks/compute/config/ProfitBricksComputeServiceContextModule.java (4)
M providers/profitbricks/src/test/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceLiveTest.java (36)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/997.patch
https://github.com/jclouds/jclouds/pull/997.diff
--
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/pull/997
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -46,9 +46,12 @@ protected ArbitraryCpuRamTemplateBuilderImpl(@Memoized Supplier<Set<? extends Lo
> super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> }
>
> - protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> - return new HardwareBuilder()
> - .id(automaticHardwareIdSpecBuilder(cores, ram).toString())
> + protected Hardware automaticHardwareForCpuAndRam(double cores, int ram, float diskSize) {
Rename to just `automaticHardware` or `automaticHardwareForCpuRamAndDisk`.
--
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/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c#r74835617
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> AutomaticHardwareIdSpec spec = new AutomaticHardwareIdSpec();
> - if (cores == 0 || ram == 0) {
> - throw new IllegalArgumentException(String.format("Omitted or wrong minCores and minRam. If you" +
> - " want to use exact values, please set the minCores and minRam values."));
> + if (cores <= 0 || ram == 0) {
> + throw new IllegalArgumentException(String.format("Omitted or wrong minCores and minRam. If you want to" +
> + " use exact values, please set the minCores and minRam values: cores=%s, ram=%s", cores, ram));
> + }
> + if (disk.isPresent() && disk.get() <= 0.0f) {
> + throw new IllegalArgumentException(String.format("Invalid disk value: %.0f", disk.get()));
Fix indentation.
--
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/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf#r74858700
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -46,9 +46,12 @@ protected ArbitraryCpuRamTemplateBuilderImpl(@Memoized Supplier<Set<? extends Lo
> super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> }
>
> - protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> - return new HardwareBuilder()
> - .id(automaticHardwareIdSpecBuilder(cores, ram).toString())
> + protected Hardware automaticHardwareForCpuAndRam(double cores, int ram, float diskSize) {
Also, the "disk" parameter is optional. Better use a `Float` object to allow to pass null, or even better an `Optional<Float>`, so callers know exactly what the contract of the method is.
--
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/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c#r74835976
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -46,9 +47,14 @@ protected ArbitraryCpuRamTemplateBuilderImpl(@Memoized Supplier<Set<? extends Lo
> super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> }
>
> - protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> - return new HardwareBuilder()
> - .id(automaticHardwareIdSpecBuilder(cores, ram).toString())
> + protected Hardware automaticHardware(double cores, int ram, Optional<Float> diskSize) {
> + HardwareBuilder builder = new HardwareBuilder();
> + if (diskSize.isPresent()) {
> + if (diskSize.get() > 0.0f)
Do it in one line ` if (diskSize.isPresent() && diskSize.get() > 0.0f)`.
--
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/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf#r74858373
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -46,9 +47,14 @@ protected ArbitraryCpuRamTemplateBuilderImpl(@Memoized Supplier<Set<? extends Lo
> super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> }
>
> - protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> - return new HardwareBuilder()
> - .id(automaticHardwareIdSpecBuilder(cores, ram).toString())
> + protected Hardware automaticHardware(double cores, int ram, Optional<Float> diskSize) {
> + HardwareBuilder builder = new HardwareBuilder();
> + if (diskSize.isPresent()) {
> + if (diskSize.get() > 0.0f)
> + builder.volume(new VolumeImpl(diskSize.get(), true, true));
> + }
> + return builder
> + .id(automaticHardwareIdSpecBuilder(cores, ram, diskSize.or(Optional.<Float>absent())).toString())
This is the same as passing just `diskSize`.
--
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/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf#r74858487
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> + Provider<TemplateOptions> optionsProvider = createMock(Provider.class);
> + Provider<TemplateBuilder> templateBuilderProvider = createMock(Provider.class);
> + TemplateBuilder defaultTemplate = createMock(TemplateBuilder.class);
> + GetImageStrategy getImageStrategy = createMock(GetImageStrategy.class);
> +
> + expect(optionsProvider.get()).andReturn(new TemplateOptions());
> + expect(getImageStrategy.getImage(anyObject(String.class))).andReturn(null);
> + replay(defaultTemplate, optionsProvider, templateBuilderProvider, getImageStrategy);
> + TemplateBuilderImpl templateBuilder = new ArbitraryCpuRamTemplateBuilderImpl(locations,
> + new ImageCacheSupplier(images, 60,
> + Atomics.<AuthorizationException>newReference(), Providers.of(getImageStrategy)), hardwares,
> + Suppliers.ofInstance(region), optionsProvider, templateBuilderProvider);
> + templateBuilder.minCores(2);
> + templateBuilder.minRam(4096);
> + float f = 100;
> + f = -f;
There is no need for these variables, just pass the value directly below.
--
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/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf#r74858783
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
Merged to gsoc2016-ivan as [0b877b30](http://git-wip-us.apache.org/repos/asf/jclouds/commit/0b877b30).
--
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/pull/997#issuecomment-239971457
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 1 commit.
201cb1f Fix PR comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf..201cb1f32fc4e6ad2f27d0bd0d9f44a4eadcb540
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 4 commits.
14f3e53 Optionals
6c33749 Optionals
0113405 Optionals
6b80a16 Fix PR comments
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/997/files/68eb62ac679fe5bb71db17a6675b95e4fd931a1a..6b80a1630d9902938c483530bd65d916fbb760bf
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> + assertThat(node.getHardware().getVolumes().get(0).getSize()).isEqualTo(10);
> + }
> + finally {
> + client.destroyNodesMatching(inGroup(group + "custom"));
> + }
> + }
> +
> + @Test(dataProvider = "onlyIfAutomaticHardwareSupported", groups = {"integration", "live"})
> + public void testCreateNodeWithCustomHardwareUsingMins() throws Exception {
> + Template template = buildTemplate(templateBuilder()
> + .minCores(2).minRam(2048).minDisk(10));
> + try {
> + NodeMetadata node = getOnlyElement(client.createNodesInGroup(group + "custom", 1, template));
> + assertThat(node.getHardware().getRam()).isEqualTo(2048);
> + assertThat(node.getHardware().getProcessors().get(0).getCores()).isEqualTo(2);
> + assertThat(node.getHardware().getVolumes().get(0).getSize()).isEqualTo(10);
Add an assertion to check the hardwre id.
--
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/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c#r74837111
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> spec.cores = cores;
> spec.ram = ram;
> return spec;
> }
>
> @Override
> public String toString() {
> - return String.format("automatic:cores=%s;ram=%s", cores, ram);
> + if (disk.isPresent()) {
> + return String.format("automatic:cores=%s;ram=%s;disk=%.0f", cores, ram, disk.get().floatValue());
Fix indentation.
--
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/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf#r74858714
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
>
> protected Hardware resolveHardware(Set<? extends Hardware> hardwarel, final Iterable<? extends Image> images) {
> try {
> return super.resolveHardware(hardwarel, images);
> }
> catch (NoSuchElementException ex) {
> - if (super.minCores != 0 && super.minRam != 0) {
> - return automaticHardwareForCpuAndRam(minCores, minRam);
> + if (minCores <= 0 || minRam == 0 || minDisk < 0) {
> + throw new IllegalArgumentException("Please, set minCores, minRam and minDisk to positive values.");
Keep the first part of the original message too:
`"No hardware profile matching the given criteria was found. If you want to use exact values, please set the minCores, minRam and minDisk to positive values"`
--
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/pull/997/files/6b80a1630d9902938c483530bd65d916fbb760bf#r74858590
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> }
> +
> + @Test(expectedExceptions = IllegalArgumentException.class,
> + expectedExceptionsMessageRegExp = "Omitted or wrong minCores and minRam. If you want to" +
> + " use exact values, please set the minCores and minRam values: cores=2.0, ram=0")
> + public void automaticHardwareIdSpecBuilderWrongSpecsTest() {
> + AutomaticHardwareIdSpec.automaticHardwareIdSpecBuilder(2.0, 0, 0.0f);
> + }
> +
> + @Test(expectedExceptions = IllegalArgumentException.class,
> + expectedExceptionsMessageRegExp = "Invalid disk value: -10")
> + public void automaticHardwareIdSpecBuilderWrongDiskTest() {
> + float disk = 10;
No need for this variable. Pass the value directly.
--
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/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c#r74836860
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Iván Lomba <no...@github.com>.
@ivanlomba pushed 1 commit.
68eb62a Fix PR commets
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c..68eb62ac679fe5bb71db17a6675b95e4fd931a1a
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -46,9 +46,12 @@ protected ArbitraryCpuRamTemplateBuilderImpl(@Memoized Supplier<Set<? extends Lo
> super(locations, images, hardwares, defaultLocation, optionsProvider, defaultTemplateProvider);
> }
>
> - protected Hardware automaticHardwareForCpuAndRam(double cores, int ram) {
> - return new HardwareBuilder()
> - .id(automaticHardwareIdSpecBuilder(cores, ram).toString())
> + protected Hardware automaticHardwareForCpuAndRam(double cores, int ram, float diskSize) {
> + HardwareBuilder builder = new HardwareBuilder();
> + if (diskSize != 0.0f)
Better compare as `diskSize >= 0.0f`?
--
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/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c#r74835681
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -70,4 +75,35 @@ protected void checkOsMatchesTemplate(NodeMetadata node) {
> // Not enough description from API to match template
> }
>
> + @Override
> + @Test(dataProvider = "onlyIfAutomaticHardwareSupported", groups = {"integration", "live"})
> + public void testCreateNodeWithCustomHardware() throws Exception {
> + Template template = buildTemplate(templateBuilder()
> + .hardwareId("automatic:cores=2;ram=2048;disk=10"));
> + try {
> + NodeMetadata node = getOnlyElement(client.createNodesInGroup(group + "custom", 1, template));
> + assertThat(node.getHardware().getRam()).isEqualTo(2048);
> + assertThat(node.getHardware().getProcessors().get(0).getCores()).isEqualTo(2);
> + assertThat(node.getHardware().getVolumes().get(0).getSize()).isEqualTo(10);
Add an assertion to check the hardwre id.
--
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/pull/997/files/eb8fdff29e2b1a31c6582f77aa8965cdc764ff2c#r74837102
Re: [jclouds/jclouds] JCLOUDS-482: Add disk to
AutomaticHardwareIdSpec and support ProfitBricks (#997)
Posted by Ignasi Barrera <no...@github.com>.
Closed #997.
--
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/pull/997#event-756398318