Use an 'options converter' to switch out the behaviour differences between basic and advanced networking when building a DeployVirtualMachineOptions instance for creating nodes.

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<NetworkType, ? extends OptionsConverter> optionsConverters(){
+      return ImmutableMap.of(
+         NetworkType.ADVANCED, new OptionsConverter.AdvancedNetworkOptionsConverter(),
+         NetworkType.BASIC, new OptionsConverter.BasicNetworkOptionsConverter());
+   }
 }

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<Long, Set<IPForwardingRule>> vmToRules;
    private final Map<String, Credentials> credentialStore;
+   private final Map<NetworkType, ? extends OptionsConverter> optionsConverters;
 
    @Inject
    public CloudStackComputeServiceAdapter(CloudStackClient client, Predicate<Long> jobComplete,
@@ -98,7 +100,7 @@ public class CloudStackComputeServiceAdapter implements
          BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
          StaticNATVirtualMachineInNetwork.Factory staticNATVMInNetwork,
          CreatePortForwardingRulesForIP setupPortForwardingRulesForIP, LoadingCache<Long, Set<IPForwardingRule>> vmToRules,
-         Map<String, Credentials> credentialStore) {
+         Map<String, Credentials> credentialStore, Map<NetworkType, ? extends OptionsConverter> 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);
+      checkState(optionsConverters.containsKey(zone.getNetworkType()), "no options converter configured for network type %s",zone.getNetworkType());
-      switch(zone.getNetworkType()) {
+      DeployVirtualMachineOptions options = DeployVirtualMachineOptions.Builder.displayName(name).name(name);
-
+      OptionsConverter optionsConverter = optionsConverters.get(zone.getNetworkType());
-         case BASIC:
+      options = optionsConverter.apply(templateOptions, networks, zoneId, 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());
-            }
-            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");
-
-      }
 
       if (templateOptions.getIpOnDefaultNetwork() != null) {
          options.ipOnDefaultNetwork(templateOptions.getIpOnDefaultNetwork());

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<Long, Network> networks, long zoneId, DeployVirtualMachineOptions options);
+
+   public static class BasicNetworkOptionsConverter implements OptionsConverter {
+      @Override
+      public DeployVirtualMachineOptions apply(CloudStackTemplateOptions templateOptions, Map<Long, Network> 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<Long, Network> 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;
+      }
+   }
+}