From f9c86860abcdf6ea1f962d788e3791357d48832c Mon Sep 17 00:00:00 2001 From: Richard Downer Date: Sun, 8 Jan 2012 22:05:18 +0200 Subject: [PATCH] Use an 'options converter' to switch out the behaviour differences between basic and advanced networking when building a DeployVirtualMachineOptions instance for creating nodes. --- ...CloudStackComputeServiceContextModule.java | 10 +++ .../CloudStackComputeServiceAdapter.java | 45 ++---------- .../compute/strategy/OptionsConverter.java | 73 +++++++++++++++++++ 3 files changed, 91 insertions(+), 37 deletions(-) create mode 100644 apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/OptionsConverter.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 8869ef3c4d..fd8336fe70 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 @@ -29,6 +29,7 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.jclouds.cloudstack.CloudStackAsyncClient; import org.jclouds.cloudstack.CloudStackClient; @@ -39,8 +40,10 @@ 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.CloudStackComputeServiceAdapter; +import org.jclouds.cloudstack.compute.strategy.OptionsConverter; import org.jclouds.cloudstack.domain.IPForwardingRule; import org.jclouds.cloudstack.domain.Network; +import org.jclouds.cloudstack.domain.NetworkType; import org.jclouds.cloudstack.domain.OSType; import org.jclouds.cloudstack.domain.ServiceOffering; import org.jclouds.cloudstack.domain.Template; @@ -198,4 +201,11 @@ public class CloudStackComputeServiceContextModule } } + @Provides + @Singleton + Map optionsConverters(){ + return ImmutableMap.of( + NetworkType.ADVANCED, new OptionsConverter.AdvancedNetworkOptionsConverter(), + NetworkType.BASIC, new OptionsConverter.BasicNetworkOptionsConverter()); + } } \ No newline at end of file diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java index 28f3b8dde5..70313527a1 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java @@ -20,6 +20,7 @@ package org.jclouds.cloudstack.compute.strategy; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Predicates.and; import static com.google.common.collect.Iterables.filter; import static com.google.common.collect.Iterables.getOnlyElement; @@ -91,6 +92,7 @@ public class CloudStackComputeServiceAdapter implements private final CreatePortForwardingRulesForIP setupPortForwardingRulesForIP; private final LoadingCache> vmToRules; private final Map credentialStore; + private final Map optionsConverters; @Inject public CloudStackComputeServiceAdapter(CloudStackClient client, Predicate jobComplete, @@ -98,7 +100,7 @@ public class CloudStackComputeServiceAdapter implements BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult, StaticNATVirtualMachineInNetwork.Factory staticNATVMInNetwork, CreatePortForwardingRulesForIP setupPortForwardingRulesForIP, LoadingCache> vmToRules, - Map credentialStore) { + Map credentialStore, Map optionsConverters) { this.client = checkNotNull(client, "client"); this.jobComplete = checkNotNull(jobComplete, "jobComplete"); this.networkSupplier = checkNotNull(networkSupplier, "networkSupplier"); @@ -108,6 +110,7 @@ public class CloudStackComputeServiceAdapter implements this.setupPortForwardingRulesForIP = checkNotNull(setupPortForwardingRulesForIP, "setupPortForwardingRulesForIP"); this.vmToRules = checkNotNull(vmToRules, "vmToRules"); this.credentialStore = checkNotNull(credentialStore, "credentialStore"); + this.optionsConverters = optionsConverters; } @Override @@ -125,42 +128,10 @@ public class CloudStackComputeServiceAdapter implements CloudStackTemplateOptions templateOptions = template.getOptions().as(CloudStackTemplateOptions.class); - DeployVirtualMachineOptions options = displayName(name).name(name); - switch(zone.getNetworkType()) { - - case BASIC: - // 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()); - } - break; - - case ADVANCED: - // 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()); - } - } - break; - - default: - throw new IllegalStateException("Zone networking type is unrecognized"); - - } + checkState(optionsConverters.containsKey(zone.getNetworkType()), "no options converter configured for network type %s",zone.getNetworkType()); + DeployVirtualMachineOptions options = DeployVirtualMachineOptions.Builder.displayName(name).name(name); + OptionsConverter optionsConverter = optionsConverters.get(zone.getNetworkType()); + options = optionsConverter.apply(templateOptions, networks, zoneId, options); if (templateOptions.getIpOnDefaultNetwork() != null) { options.ipOnDefaultNetwork(templateOptions.getIpOnDefaultNetwork()); 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 new file mode 100644 index 0000000000..6ae7f2b222 --- /dev/null +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/OptionsConverter.java @@ -0,0 +1,73 @@ +/** + * 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; + +public interface OptionsConverter { + 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; + } + } +}