Repository: brooklyn-server Updated Branches: refs/heads/master 601ea0d2e -> 7b5d367b8
JCLOUDS-1108 workaround: GCE hardwareId short-form Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/c5e41af2 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/c5e41af2 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/c5e41af2 Branch: refs/heads/master Commit: c5e41af2e8113113c1c6908ca43edbf29575963c Parents: 601ea0d Author: Aled Sage <aled.s...@gmail.com> Authored: Mon Dec 19 18:08:04 2016 +0000 Committer: Aled Sage <aled.s...@gmail.com> Committed: Thu Dec 22 12:37:55 2016 +0000 ---------------------------------------------------------------------- .../location/jclouds/JcloudsLocation.java | 55 ++++++- .../jclouds/AbstractJcloudsLiveTest.java | 1 - ...loudsGceHardwareProfilesStubbedLiveTest.java | 156 +++++++++++++++++++ 3 files changed, 210 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5e41af2/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java index 67aa826..26c89fd 100644 --- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java +++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java @@ -1369,6 +1369,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im templateBuilder.locationId(config.get(CLOUD_REGION_ID)); } + if (Strings.isNonBlank(config.get(HARDWARE_ID))) { + String oldHardwareId = config.get(HARDWARE_ID); + String newHardwareId = transformHardwareId(oldHardwareId, config); + if (!Objects.equal(oldHardwareId, newHardwareId)) { + LOG.info("Transforming hardwareId from " + oldHardwareId + " to " + newHardwareId + ", in " + toString()); + config.put(HARDWARE_ID, newHardwareId); + } + } + // Apply the template builder and options properties for (Map.Entry<ConfigKey<?>, ? extends TemplateBuilderCustomizer> entry : SUPPORTED_TEMPLATE_BUILDER_PROPERTIES.entrySet()) { ConfigKey<?> key = entry.getKey(); @@ -1484,6 +1493,51 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im return template; } + + /** + * See {@link https://issues.apache.org/jira/browse/JCLOUDS-1108}. + * + * In jclouds 1.9.x and 2.0.0, google-compute-engine hardwareId must be in the long form. For + * example {@code https://www.googleapis.com/compute/v1/projects/jclouds-gce/zones/us-central1-a/machineTypes/n1-standard-1}. + * It is much nicer to support the short-form (e.g. {@code n1-standard-1}), and to construct + * the long-form from this. + * + * The "zone" in the long-form needs to match the region (see {@link #getRegion()}). + * + * The ideal would be for jclouds to do this. But that isn't available yet - in the mean time, + * we can make life easier for our users with the code below. + * + * Second best would have been handling this in {@link TemplateBuilderCustomizers#hardwareId()}. + * However, that code doesn't have enough context to know what to do (easily!). It is passed + * {@code apply(TemplateBuilder tb, ConfigBag props, Object v)}, so doesn't even know which + * provider it is being called for (without doing ugly/brittle digging in the {@code props} + * that it is given). + * + * Therefore we do the transform here. + */ + private String transformHardwareId(String hardwareId, ConfigBag config) { + checkNotNull(hardwareId, "hardwareId"); + checkNotNull(config, "config"); + + String provider = getProvider(); + String region = getRegion(); + if (Strings.isBlank(region)) region = config.get(CLOUD_REGION_ID); + + if (!"google-compute-engine".equals(provider)) { + return hardwareId; + } + if (hardwareId.toLowerCase().startsWith("http") || hardwareId.contains("/")) { + // looks like it's already in long-form: don't transform + return hardwareId; + } + if (Strings.isNonBlank(region)) { + return String.format("https://www.googleapis.com/compute/v1/projects/jclouds-gce/zones/%s/machineTypes/%s";, region, hardwareId); + } else { + LOG.warn("Cannot transform GCE hardwareId (" + hardwareId + ") to long form, because region unknown in " + toString()); + return hardwareId; + } + } + protected String toStringNice() { String s = config().get(ORIGINAL_SPEC); if (Strings.isBlank(s)) s = config().get(NAMED_SPEC_NAME); @@ -3051,5 +3105,4 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im return val1; } } - } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5e41af2/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/AbstractJcloudsLiveTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/AbstractJcloudsLiveTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/AbstractJcloudsLiveTest.java index 706d308..a4d4c42 100644 --- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/AbstractJcloudsLiveTest.java +++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/AbstractJcloudsLiveTest.java @@ -22,7 +22,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; -import java.net.InetAddress; import java.util.List; import java.util.Map; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/c5e41af2/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsGceHardwareProfilesStubbedLiveTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsGceHardwareProfilesStubbedLiveTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsGceHardwareProfilesStubbedLiveTest.java new file mode 100644 index 0000000..ba2128d --- /dev/null +++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsGceHardwareProfilesStubbedLiveTest.java @@ -0,0 +1,156 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software 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.apache.brooklyn.location.jclouds; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +import java.util.NoSuchElementException; + +import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.BasicNodeCreator; +import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.NodeCreator; +import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.jclouds.compute.domain.NodeMetadata; +import org.jclouds.compute.domain.Template; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableMap; + +public class JcloudsGceHardwareProfilesStubbedLiveTest extends AbstractJcloudsStubbedLiveTest { + + @SuppressWarnings("unused") + private static final Logger log = LoggerFactory.getLogger(JcloudsGceHardwareProfilesStubbedLiveTest.class); + + private static final String LOCATION_SPEC = GCE_PROVIDER + ":" + GCE_USCENTRAL_REGION_NAME; + + private static final String N1_STANDARD_1_HARDWARE_ID = "n1-standard-1"; + private static final String G1_SMALL_HARDWARE_ID = "g1-small"; + private static final String N1_STANDARD_2_HARDWARE_ID = "n1-standard-2"; + private static final String N1_HIGHCPU_4_HARDWARE_ID = "n1-highcpu-4"; + + private static final String GCE_ZONES_PREFIX = "https://www.googleapis.com/compute/v1/projects/jclouds-gce/zones/";; + private static final String G1_SMALL_HARDWARE_ID_LONG_FORM = GCE_ZONES_PREFIX + GCE_USCENTRAL_REGION_NAME + "/machineTypes/" + G1_SMALL_HARDWARE_ID; + private static final String N1_STANDARD_1_HARDWARE_ID_LONG_FORM = GCE_ZONES_PREFIX + GCE_USCENTRAL_REGION_NAME + "/machineTypes/" + N1_STANDARD_1_HARDWARE_ID; + private static final String N1_STANDARD_2_HARDWARE_ID_LONG_FORM = GCE_ZONES_PREFIX + GCE_USCENTRAL_REGION_NAME + "/machineTypes/" + N1_STANDARD_2_HARDWARE_ID; + private static final String N1_HIGHCPU_4_HARDWARE_ID_LONG_FORM = GCE_ZONES_PREFIX + GCE_USCENTRAL_REGION_NAME + "/machineTypes/" + N1_HIGHCPU_4_HARDWARE_ID; + + private Template template; + + @Override + protected String getLocationSpec() { + return LOCATION_SPEC; + } + + @Override + protected NodeCreator newNodeCreator() { + return new BasicNodeCreator() { + @Override protected NodeMetadata newNode(String group, Template template) { + JcloudsGceHardwareProfilesStubbedLiveTest.this.template = template; + return super.newNode(group, template); + } + }; + } + + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareProfiles() throws Exception { + // default minRam is 1gb + obtainMachine(); + assertTrue(template.getHardware().getRam() >= 1000, "template="+template); + assertEquals(template.getHardware().getId(), G1_SMALL_HARDWARE_ID_LONG_FORM, "template="+template); + + obtainMachine(MutableMap.of(JcloudsLocationConfig.MIN_RAM, "4096")); + assertTrue(template.getHardware().getRam() >= 4096, "template="+template); + assertEquals(template.getHardware().getId(), N1_STANDARD_2_HARDWARE_ID_LONG_FORM, "template="+template); + + obtainMachine(MutableMap.of(JcloudsLocationConfig.MIN_CORES, "4")); + assertTrue(template.getHardware().getProcessors().get(0).getCores() >= 4, "template="+template); + assertEquals(template.getHardware().getId(), N1_HIGHCPU_4_HARDWARE_ID_LONG_FORM, "template="+template); + } + + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareIdLongForm() throws Exception { + obtainMachine(ImmutableMap.of(JcloudsLocation.HARDWARE_ID, N1_STANDARD_1_HARDWARE_ID_LONG_FORM)); + + assertEquals(template.getHardware().getId(), N1_STANDARD_1_HARDWARE_ID_LONG_FORM, "template="+template); + } + + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareIdLongFormWithNoRegionInLocationSpec() throws Exception { + // region will be passed in the obtain() call + replaceJcloudsLocation(GCE_PROVIDER); + + obtainMachine(ImmutableMap.of( + JcloudsLocation.CLOUD_REGION_ID, GCE_USCENTRAL_REGION_NAME, + JcloudsLocation.HARDWARE_ID, N1_STANDARD_1_HARDWARE_ID_LONG_FORM)); + + assertEquals(template.getHardware().getId(), N1_STANDARD_1_HARDWARE_ID_LONG_FORM, "template="+template); + } + + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareIdLongFormWithNoRegion() throws Exception { + // Expect it to default to GCE_USCENTRAL_REGION_NAME - + // but is that only because that region is hard-coded in the long hardwareId?! + replaceJcloudsLocation(GCE_PROVIDER); + + obtainMachine(ImmutableMap.of(JcloudsLocation.HARDWARE_ID, N1_STANDARD_1_HARDWARE_ID_LONG_FORM)); + + assertEquals(template.getHardware().getId(), N1_STANDARD_1_HARDWARE_ID_LONG_FORM, "template="+template); + } + + // See https://issues.apache.org/jira/browse/JCLOUDS-1108 + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareIdShortForm() throws Exception { + obtainMachine(ImmutableMap.of(JcloudsLocation.HARDWARE_ID, N1_STANDARD_1_HARDWARE_ID)); + + assertEquals(template.getHardware().getId(), N1_STANDARD_1_HARDWARE_ID_LONG_FORM, "template="+template); + } + + // See https://issues.apache.org/jira/browse/JCLOUDS-1108 + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareIdShortFormWithNoRegionInLocationSpec() throws Exception { + replaceJcloudsLocation(GCE_PROVIDER); + + obtainMachine(ImmutableMap.of( + JcloudsLocation.CLOUD_REGION_ID, GCE_USCENTRAL_REGION_NAME, + JcloudsLocation.HARDWARE_ID, N1_STANDARD_1_HARDWARE_ID)); + + assertEquals(template.getHardware().getId(), N1_STANDARD_1_HARDWARE_ID_LONG_FORM, "template="+template); + } + + // See https://issues.apache.org/jira/browse/JCLOUDS-1108 + // Region needs to be specified somewhere. + @Test(groups={"Live", "Live-sanity"}) + public void testJcloudsCreateWithHardwareIdShortFormWithNoRegion() throws Exception { + replaceJcloudsLocation(GCE_PROVIDER); + + try { + obtainMachine(ImmutableMap.of(JcloudsLocation.HARDWARE_ID, N1_STANDARD_1_HARDWARE_ID)); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + NoSuchElementException nsee = Exceptions.getFirstThrowableOfType(e, NoSuchElementException.class); + if (nsee == null || !nsee.toString().contains("hardwareId("+N1_STANDARD_1_HARDWARE_ID+") not found")) { + throw e; + } + } + } +}
