Azure ARM default network fixes from PR suggestions
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a11fd427 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a11fd427 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a11fd427 Branch: refs/heads/master Commit: a11fd427b0ec94748516867194d8531d6966e784 Parents: 3d8675e Author: graeme.miller <[email protected]> Authored: Fri Jun 23 12:53:41 2017 +0100 Committer: graeme.miller <[email protected]> Committed: Tue Jul 4 11:34:58 2017 +0100 ---------------------------------------------------------------------- .../creator/DefaultAzureArmNetworkCreator.java | 56 ++++++++++----- .../DefaultAzureArmNetworkCreatorTest.java | 76 +++++++++++++++----- 2 files changed, 96 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a11fd427/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreator.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreator.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreator.java index dc87cfc..d08bd03 100644 --- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreator.java +++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreator.java @@ -26,15 +26,19 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.jclouds.azurecompute.arm.AzureComputeApi; import org.jclouds.azurecompute.arm.domain.ResourceGroup; import org.jclouds.azurecompute.arm.domain.Subnet; import org.jclouds.azurecompute.arm.domain.VirtualNetwork; +import org.jclouds.azurecompute.arm.features.SubnetApi; +import org.jclouds.azurecompute.arm.features.VirtualNetworkApi; import org.jclouds.compute.ComputeService; import org.apache.brooklyn.config.ConfigKey; @@ -64,29 +68,36 @@ public class DefaultAzureArmNetworkCreator { LOG.debug("azure.arm.default.network.enabled is disabled, not creating default network"); return; } - + String location = config.get(CLOUD_REGION_ID); + if(StringUtils.isEmpty(location)) { + LOG.debug("No region information, so cannot create a default network"); + return; + } Map<String, Object> templateOptions = config.get(TEMPLATE_OPTIONS); + //Only create a default network if we haven't specified a network name (in template options or config) or ip options if (config.containsKey(NETWORK_NAME)) { - LOG.debug("Network config specified when provisioning Azure machine. Not creating default network"); + LOG.debug("Network config [{}] specified when provisioning Azure machine. Not creating default network", NETWORK_NAME.getName()); return; } - if (templateOptions != null && (templateOptions.containsKey(NETWORK_NAME.getName()) || templateOptions.containsKey("ipOptions"))) { + if (templateOptions != null && (templateOptions.containsKey("networks") || templateOptions.containsKey("ipOptions"))) { LOG.debug("Network config specified when provisioning Azure machine. Not creating default network"); return; } AzureComputeApi api = computeService.getContext().unwrapApi(AzureComputeApi.class); - String location = config.get(CLOUD_REGION_ID); String resourceGroupName = DEFAULT_RESOURCE_GROUP_PREFIX + "-" + location; String vnetName = DEFAULT_NETWORK_NAME_PREFIX + "-" + location; String subnetName = DEFAULT_SUBNET_NAME_PREFIX + "-" + location; + SubnetApi subnetApi = api.getSubnetApi(resourceGroupName, vnetName); + VirtualNetworkApi virtualNetworkApi = api.getVirtualNetworkApi(resourceGroupName); + //Check if default already exists - Subnet preexistingSubnet = api.getSubnetApi(resourceGroupName, vnetName).get(subnetName); + Subnet preexistingSubnet = subnetApi.get(subnetName); if(preexistingSubnet != null){ LOG.info("Using pre-existing default Azure network [{}] and subnet [{}] when provisioning machine", vnetName, subnetName); @@ -94,27 +105,34 @@ public class DefaultAzureArmNetworkCreator { return; } + createResourceGroupIfNeeded(api, resourceGroupName, location); - LOG.info("Network config not specified when provisioning Azure machine, and default network/subnet does not exists. " - + "Creating network [{}] and subnet [{}], and updating template options", vnetName, subnetName); + Subnet.SubnetProperties subnetProperties = Subnet.SubnetProperties.builder().addressPrefix(DEFAULT_SUBNET_ADDRESS_PREFIX).build(); - createResourceGroupIfNeeded(api, resourceGroupName, location); + //Creating network + subnet if network doesn't exist + if(virtualNetworkApi.get(vnetName) == null) { + LOG.info("Network config not specified when provisioning Azure machine, and default network/subnet does not exists. " + + "Creating network [{}] and subnet [{}], and updating template options", vnetName, subnetName); - //Setup properties for creating subnet/network - Subnet subnet = Subnet.create(subnetName, null, null, - Subnet.SubnetProperties.builder().addressPrefix(DEFAULT_SUBNET_ADDRESS_PREFIX).build()); + Subnet subnet = Subnet.create(subnetName, null, null, subnetProperties); - VirtualNetwork.VirtualNetworkProperties virtualNetworkProperties = VirtualNetwork.VirtualNetworkProperties - .builder().addressSpace(VirtualNetwork.AddressSpace.create(Arrays.asList(DEFAULT_VNET_ADDRESS_PREFIX))) - .subnets(Arrays.asList(subnet)).build(); + VirtualNetwork.VirtualNetworkProperties virtualNetworkProperties = VirtualNetwork.VirtualNetworkProperties + .builder().addressSpace(VirtualNetwork.AddressSpace.create(Arrays.asList(DEFAULT_VNET_ADDRESS_PREFIX))) + .subnets(Arrays.asList(subnet)).build(); + virtualNetworkApi.createOrUpdate(vnetName, location, virtualNetworkProperties); + } else { + LOG.info("Network config not specified when provisioning Azure machine, and default subnet does not exists. " + + "Creating subnet [{}] on network [{}], and updating template options", subnetName, vnetName); - //Create network - api.getVirtualNetworkApi(resourceGroupName).createOrUpdate(vnetName, location, virtualNetworkProperties); + //Just creating the subnet + subnetApi.createOrUpdate(subnetName, subnetProperties); + } + + //Get created subnet Subnet createdSubnet = api.getSubnetApi(resourceGroupName, vnetName).get(subnetName); //Add config updateTemplateOptions(config, createdSubnet); - } private static void updateTemplateOptions(ConfigBag config, Subnet createdSubnet){ @@ -126,10 +144,10 @@ public class DefaultAzureArmNetworkCreator { templateOptions = new HashMap<>(); } - templateOptions.put("ipOptions", ImmutableMap.of( + templateOptions.put("ipOptions", ImmutableList.of(ImmutableMap.of( "allocateNewPublicIp", true, //jclouds will not provide a public IP unless we set this "subnet", createdSubnet.id() - )); + ))); config.put(TEMPLATE_OPTIONS, templateOptions); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a11fd427/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreatorTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreatorTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreatorTest.java index 86a5778..48d6024 100644 --- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreatorTest.java +++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/creator/DefaultAzureArmNetworkCreatorTest.java @@ -31,12 +31,14 @@ import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.brooklyn.util.core.config.ConfigBag; import org.jclouds.azurecompute.arm.AzureComputeApi; import org.jclouds.azurecompute.arm.domain.ResourceGroup; import org.jclouds.azurecompute.arm.domain.Subnet; +import org.jclouds.azurecompute.arm.domain.VirtualNetwork; import org.jclouds.azurecompute.arm.features.ResourceGroupApi; import org.jclouds.azurecompute.arm.features.SubnetApi; import org.jclouds.azurecompute.arm.features.VirtualNetworkApi; @@ -58,6 +60,7 @@ public class DefaultAzureArmNetworkCreatorTest { @Mock ResourceGroupApi resourceGroupApi; @Mock VirtualNetworkApi virtualNetworkApi; @Mock SubnetApi subnetApi; + @Mock VirtualNetwork virtualNetwork; @Mock ResourceGroup resourceGroup; @Mock Subnet subnet; @@ -89,7 +92,6 @@ public class DefaultAzureArmNetworkCreatorTest { //Setup mocks when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(subnet); - //Test DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); @@ -100,7 +102,7 @@ public class DefaultAzureArmNetworkCreatorTest { //verify templateOptions updated to include defaults Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); - Map<?, ?> ipOptions = (Map<?, ?>)templateOptions.get("ipOptions"); + Map<String, Object> ipOptions = (Map<String, Object>) ((List)templateOptions.get("ipOptions")).iterator().next(); assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); assertEquals(ipOptions.get("allocateNewPublicIp"), true); } @@ -121,9 +123,9 @@ public class DefaultAzureArmNetworkCreatorTest { protected ConfigBag runVanilla(Map<?, ?> additionalConfig) throws Exception { //Setup config bag ConfigBag configBag = ConfigBag.newInstance(); - configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.putAll(additionalConfig); - + configBag.put(CLOUD_REGION_ID, TEST_LOCATION); + //Setup mocks when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(null).thenReturn(subnet); //null first time, subnet next @@ -144,7 +146,7 @@ public class DefaultAzureArmNetworkCreatorTest { //verify templateOptions updated to include defaults Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); - Map<?, ?> ipOptions = (Map<?, ?>)templateOptions.get("ipOptions"); + Map<String, Object> ipOptions = (Map<String, Object>) ((List)templateOptions.get("ipOptions")).iterator().next(); assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); assertEquals(ipOptions.get("allocateNewPublicIp"), true); @@ -156,10 +158,9 @@ public class DefaultAzureArmNetworkCreatorTest { //Setup config bag ConfigBag configBag = ConfigBag.newInstance(); configBag.put(CLOUD_REGION_ID, TEST_LOCATION); - + //Setup mocks when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(null).thenReturn(subnet); //null first time, subnet next - when(resourceGroupApi.get(TEST_RESOURCE_GROUP)).thenReturn(resourceGroup); @@ -177,7 +178,41 @@ public class DefaultAzureArmNetworkCreatorTest { //verify templateOptions updated to include defaults Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); - Map<?, ?> ipOptions = (Map<?, ?>)templateOptions.get("ipOptions"); + Map<String, Object> ipOptions = (Map<String, Object>) ((List)templateOptions.get("ipOptions")).iterator().next(); + assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); + assertEquals(ipOptions.get("allocateNewPublicIp"), true); + } + + + + @Test + public void testVanillaWhereExistingNetworkButNoSubnet() throws Exception { + //Setup config bag + ConfigBag configBag = ConfigBag.newInstance(); + configBag.put(CLOUD_REGION_ID, TEST_LOCATION); + + //Setup mocks + when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(null).thenReturn(subnet); //null first time, subnet next + when(virtualNetworkApi.get(TEST_NETWORK_NAME)).thenReturn(virtualNetwork); + when(resourceGroupApi.get(TEST_RESOURCE_GROUP)).thenReturn(resourceGroup); + + + //Test + DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); + + //verify + verify(subnetApi, times(2)).get(TEST_SUBNET_NAME); + verify(subnetApi).createOrUpdate(eq(TEST_SUBNET_NAME), any()); + verify(subnet).id(); + + verify(resourceGroupApi).get(TEST_RESOURCE_GROUP); + verify(resourceGroupApi, never()).create(any(), any(), any()); + + verify(virtualNetworkApi, never()).createOrUpdate(any(), any(), any()); + + //verify templateOptions updated to include defaults + Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); + Map<String, Object> ipOptions = (Map<String, Object>) ((List)templateOptions.get("ipOptions")).iterator().next(); assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); assertEquals(ipOptions.get("allocateNewPublicIp"), true); } @@ -188,19 +223,19 @@ public class DefaultAzureArmNetworkCreatorTest { configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(NETWORK_NAME, TEST_NETWORK_NAME); - runAssertingNoIteractions(configBag); + runAssertingNoInteractions(configBag); } @Test public void testNetworkInTemplate() throws Exception { HashMap<String, Object> templateOptions = new HashMap<>(); - templateOptions.put(NETWORK_NAME.getName(), TEST_NETWORK_NAME); + templateOptions.put("networks", TEST_NETWORK_NAME); ConfigBag configBag = ConfigBag.newInstance(); - configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(TEMPLATE_OPTIONS, templateOptions); + configBag.put(CLOUD_REGION_ID, TEST_LOCATION); - runAssertingNoIteractions(configBag); + runAssertingNoInteractions(configBag); } @Test @@ -209,22 +244,29 @@ public class DefaultAzureArmNetworkCreatorTest { templateOptions.put("ipOptions", TEST_NETWORK_NAME); ConfigBag configBag = ConfigBag.newInstance(); - configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(TEMPLATE_OPTIONS, templateOptions); + configBag.put(CLOUD_REGION_ID, TEST_LOCATION); - runAssertingNoIteractions(configBag); + runAssertingNoInteractions(configBag); } @Test public void testConfigDisabled() throws Exception { ConfigBag configBag = ConfigBag.newInstance(); - configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(AZURE_ARM_DEFAULT_NETWORK_ENABLED, false); + configBag.put(CLOUD_REGION_ID, TEST_LOCATION); + + runAssertingNoInteractions(configBag); + } + + @Test + public void testNoRegion() throws Exception { + ConfigBag configBag = ConfigBag.newInstance(); - runAssertingNoIteractions(configBag); + runAssertingNoInteractions(configBag); } - protected void runAssertingNoIteractions(ConfigBag configBag) throws Exception { + protected void runAssertingNoInteractions(ConfigBag configBag) throws Exception { Map<String, Object> configCopy = configBag.getAllConfig(); DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag);
