From ce6b79eeca82cb0ad44507c56e2a7097aa92efac Mon Sep 17 00:00:00 2001 From: Richard Downer Date: Mon, 9 Jan 2012 15:07:36 +0200 Subject: [PATCH] Move the inner classes of OptionsConverter into own classes. Add JavaDoc and unit test. --- ...CloudStackComputeServiceContextModule.java | 6 +- .../AdvancedNetworkOptionsConverter.java | 59 +++++++++ .../BasicNetworkOptionsConverter.java | 45 +++++++ .../compute/strategy/OptionsConverter.java | 53 +++----- .../strategy/OptionsConverterTest.java | 121 ++++++++++++++++++ 5 files changed, 246 insertions(+), 38 deletions(-) create mode 100644 apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/AdvancedNetworkOptionsConverter.java create mode 100644 apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/BasicNetworkOptionsConverter.java create mode 100644 apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/strategy/OptionsConverterTest.java diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java index e1633a0117..fbf3193621 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java @@ -39,6 +39,8 @@ import org.jclouds.cloudstack.compute.functions.TemplateToOperatingSystem; import org.jclouds.cloudstack.compute.functions.VirtualMachineToNodeMetadata; import org.jclouds.cloudstack.compute.functions.ZoneToLocation; import org.jclouds.cloudstack.compute.options.CloudStackTemplateOptions; +import org.jclouds.cloudstack.compute.strategy.AdvancedNetworkOptionsConverter; +import org.jclouds.cloudstack.compute.strategy.BasicNetworkOptionsConverter; import org.jclouds.cloudstack.compute.strategy.CloudStackComputeServiceAdapter; import org.jclouds.cloudstack.compute.strategy.OptionsConverter; import org.jclouds.cloudstack.domain.IPForwardingRule; @@ -211,7 +213,7 @@ public class CloudStackComputeServiceContextModule @Singleton Map optionsConverters(){ return ImmutableMap.of( - NetworkType.ADVANCED, new OptionsConverter.AdvancedNetworkOptionsConverter(), - NetworkType.BASIC, new OptionsConverter.BasicNetworkOptionsConverter()); + NetworkType.ADVANCED, new AdvancedNetworkOptionsConverter(), + NetworkType.BASIC, new BasicNetworkOptionsConverter()); } } \ No newline at end of file diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/AdvancedNetworkOptionsConverter.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/AdvancedNetworkOptionsConverter.java new file mode 100644 index 0000000000..d8aa4d2a03 --- /dev/null +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/AdvancedNetworkOptionsConverter.java @@ -0,0 +1,59 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds 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.jclouds.cloudstack.compute.strategy; + +import com.google.common.collect.Iterables; +import org.jclouds.cloudstack.compute.options.CloudStackTemplateOptions; +import org.jclouds.cloudstack.domain.Network; +import org.jclouds.cloudstack.options.DeployVirtualMachineOptions; + +import java.util.Map; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Predicates.and; +import static com.google.common.collect.Iterables.filter; +import static org.jclouds.cloudstack.predicates.NetworkPredicates.defaultNetworkInZone; +import static org.jclouds.cloudstack.predicates.NetworkPredicates.supportsStaticNAT; + +/** + * Convert template options into DeployVirtualMachineOptions, when the target zone has advanced networking. + * + * @author Richard Downer + */ +public class AdvancedNetworkOptionsConverter implements OptionsConverter { + @Override + public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptions, Map networks, long zoneId, DeployVirtualMachineOptions options) { + // security groups not allowed. + // at least one network must be given to CloudStack, + // but jclouds will try to autodetect an appropriate network if none given. + checkArgument(templateOptions.getSecurityGroupIds().isEmpty(), "security groups cannot be specified for locations (zones) that use advanced networking"); + if (templateOptions.getNetworkIds().size() > 0) { + options.networkIds(templateOptions.getNetworkIds()); + } else { + checkArgument(!networks.isEmpty(), "please setup a network for zone: " + zoneId); + Network defaultNetworkInZone = Iterables.getFirst(filter(networks.values(), and(defaultNetworkInZone(zoneId), supportsStaticNAT())), null); + if(defaultNetworkInZone == null) { + throw new IllegalArgumentException("please choose a specific network in zone " + zoneId + ": " + networks); + } else { + options.networkId(defaultNetworkInZone.getId()); + } + } + return options; + } +} diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/BasicNetworkOptionsConverter.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/BasicNetworkOptionsConverter.java new file mode 100644 index 0000000000..072de8a5b0 --- /dev/null +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/BasicNetworkOptionsConverter.java @@ -0,0 +1,45 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds 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.jclouds.cloudstack.compute.strategy; + +import org.jclouds.cloudstack.compute.options.CloudStackTemplateOptions; +import org.jclouds.cloudstack.domain.Network; +import org.jclouds.cloudstack.options.DeployVirtualMachineOptions; + +import java.util.Map; + +/** + * Convert template options into DeployVirtualMachineOptions, when the target zone has basic networking. + * + * @author Richard Downer + */ +public class BasicNetworkOptionsConverter implements OptionsConverter { + @Override + public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptions, Map networks, long zoneId, DeployVirtualMachineOptions options) { + // both security groups and networks are optional, and CloudStack will + // use the zone/user's default network/security group if none given + if (templateOptions.getSecurityGroupIds().size() > 0) { + options.securityGroupIds(templateOptions.getSecurityGroupIds()); + } + if (templateOptions.getNetworkIds().size() > 0) { + options.networkIds(templateOptions.getNetworkIds()); + } + return options; + } +} diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/OptionsConverter.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/OptionsConverter.java index 6ae7f2b222..bbd0cf5e18 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/OptionsConverter.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/OptionsConverter.java @@ -31,43 +31,24 @@ import static com.google.common.collect.Iterables.filter; import static org.jclouds.cloudstack.predicates.NetworkPredicates.defaultNetworkInZone; import static org.jclouds.cloudstack.predicates.NetworkPredicates.supportsStaticNAT; +/** + * Convert template options into DeployVirtualMachineOptions. Expressed as an interface, because in + * CloudStack different zone network types have different requirements when it comes to networks and + * security groups. + * + * @author Richard Downer + */ public interface OptionsConverter { + + /** + * Convert a CloudStackTemplateOptions and apply to a DeployVirtualMachineOptions instance. + * + * @param templateOptions the input set of options + * @param networks the networks available + * @param zoneId the zone of the new virtual machine + * @param options where the resulting set of options will be applied + * @return same as "options" parameter + */ DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptions, Map networks, long zoneId, DeployVirtualMachineOptions options); - public static class BasicNetworkOptionsConverter implements OptionsConverter { - @Override - public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptions, Map networks, long zoneId, DeployVirtualMachineOptions options) { - // both security groups and networks are optional, and CloudStack will - // use the zone/user's default network/security group if none given - if (templateOptions.getSecurityGroupIds().size() > 0) { - options.securityGroupIds(templateOptions.getSecurityGroupIds()); - } - if (templateOptions.getNetworkIds().size() > 0) { - options.networkIds(templateOptions.getNetworkIds()); - } - return options; - } - } - - public static class AdvancedNetworkOptionsConverter implements OptionsConverter { - @Override - public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptions, Map networks, long zoneId, DeployVirtualMachineOptions options) { - // security groups not allowed. - // at least one network must be given to CloudStack, - // but jclouds will try to autodetect an appropriate network if none given. - checkArgument(templateOptions.getSecurityGroupIds().isEmpty(), "security groups cannot be specified for locations (zones) that use advanced networking"); - if (templateOptions.getNetworkIds().size() > 0) { - options.networkIds(templateOptions.getNetworkIds()); - } else { - checkArgument(!networks.isEmpty(), "please setup a network for zone: " + zoneId); - Network defaultNetworkInZone = Iterables.getFirst(filter(networks.values(), and(defaultNetworkInZone(zoneId), supportsStaticNAT())), null); - if(defaultNetworkInZone == null) { - throw new IllegalArgumentException("please choose a specific network in zone " + zoneId + ": " + networks); - } else { - options.networkId(defaultNetworkInZone.getId()); - } - } - return options; - } - } } diff --git a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/strategy/OptionsConverterTest.java b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/strategy/OptionsConverterTest.java new file mode 100644 index 0000000000..b99ecc70b6 --- /dev/null +++ b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/strategy/OptionsConverterTest.java @@ -0,0 +1,121 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds 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.jclouds.cloudstack.compute.strategy; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import org.jclouds.cloudstack.compute.options.CloudStackTemplateOptions; +import org.jclouds.cloudstack.domain.Network; +import org.jclouds.cloudstack.domain.NetworkService; +import org.jclouds.cloudstack.options.DeployVirtualMachineOptions; +import org.testng.annotations.Test; + +import java.util.Collections; +import java.util.Map; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +@Test +public class OptionsConverterTest { + + private static final Map EMPTY_NETWORKS_MAP = Collections.emptyMap(); + private static final int ZONE_ID = 2; + private final NetworkService firewallServiceWithStaticNat + = new NetworkService("Firewall", ImmutableMap.of("StaticNat", "true")); + + @Test + public void testBasicNetworkOptionsConverter() { + BasicNetworkOptionsConverter converter = new BasicNetworkOptionsConverter(); + + CloudStackTemplateOptions optionsIn = CloudStackTemplateOptions.Builder.securityGroupId(42).networkId(46); + DeployVirtualMachineOptions optionsOut = DeployVirtualMachineOptions.NONE; + + DeployVirtualMachineOptions optionsOut2 = converter.apply(optionsIn, EMPTY_NETWORKS_MAP, ZONE_ID, optionsOut); + assertTrue(optionsOut == optionsOut2); + + DeployVirtualMachineOptions optionsExpected = DeployVirtualMachineOptions.Builder.securityGroupId(42).networkId(46); + assertEquals(optionsOut, optionsExpected); + } + + @Test + public void testAdvancedSecurityGroupsNotAllowed() { + boolean exceptionThrown = false; + AdvancedNetworkOptionsConverter converter = new AdvancedNetworkOptionsConverter(); + CloudStackTemplateOptions optionsIn = CloudStackTemplateOptions.Builder.securityGroupId(42); + + try { + converter.apply(optionsIn, EMPTY_NETWORKS_MAP, ZONE_ID, DeployVirtualMachineOptions.NONE); + } catch(IllegalArgumentException e) { + exceptionThrown = true; + } + + assertTrue(exceptionThrown, "IllegalArgumentException should have been thrown"); + } + + @Test + public void testAdvancedExplicitNetworkSelection() { + AdvancedNetworkOptionsConverter converter = new AdvancedNetworkOptionsConverter(); + DeployVirtualMachineOptions optionsActual = converter.apply(CloudStackTemplateOptions.Builder.networkId(42), + EMPTY_NETWORKS_MAP, ZONE_ID, DeployVirtualMachineOptions.NONE); + DeployVirtualMachineOptions optionsExpected = DeployVirtualMachineOptions.Builder.networkId(42); + assertEquals(optionsActual, optionsExpected); + } + + @Test + public void testAdvancedAutoDetectNetwork() { + AdvancedNetworkOptionsConverter converter = new AdvancedNetworkOptionsConverter(); + + Network eligibleNetwork = Network.builder() + .id(25).zoneId(ZONE_ID).isDefault(true).services(ImmutableSet.of(firewallServiceWithStaticNat)) + .build(); + DeployVirtualMachineOptions optionsActual = converter.apply(CloudStackTemplateOptions.NONE, + ImmutableMap.of(eligibleNetwork.getId(), eligibleNetwork), ZONE_ID, DeployVirtualMachineOptions.NONE); + DeployVirtualMachineOptions optionsExpected = DeployVirtualMachineOptions.Builder.networkId(25); + assertEquals(optionsActual, optionsExpected); + } + + @Test + public void testAdvancedWhenNoNetworkGiven() { + AdvancedNetworkOptionsConverter converter = new AdvancedNetworkOptionsConverter(); + boolean exceptionThrown = false; + try { + converter.apply(CloudStackTemplateOptions.NONE, EMPTY_NETWORKS_MAP, ZONE_ID, DeployVirtualMachineOptions.NONE); + } catch(IllegalArgumentException e) { + exceptionThrown = true; + } + assertTrue(exceptionThrown); + } + + @Test + public void testAdvancedWhenNoNetworkEligible() { + AdvancedNetworkOptionsConverter converter = new AdvancedNetworkOptionsConverter(); + Network unsuitableNetwork = Network.builder() + .id(25).zoneId(ZONE_ID) + .build(); + + boolean exceptionThrown = false; + try { + converter.apply(CloudStackTemplateOptions.NONE, ImmutableMap.of(unsuitableNetwork.getId(), unsuitableNetwork), ZONE_ID, DeployVirtualMachineOptions.NONE); + } catch(IllegalArgumentException e) { + exceptionThrown = true; + } + assertTrue(exceptionThrown); + } +}