Repository: brooklyn-server Updated Branches: refs/heads/master 733fe7737 -> 169b55e41
DefaultAzureArmNetworkCreator improvements * Donât use RETURNS_DEEP_STUBS as causes non-deterministic test failure on my dev machine (ClassCastException when calling `thenReturn(...)`!) * Tidy logging * Assert create() calls are made * Minor renames (e.g. _PREFIX to indicate DEFAULT_RESOURCE_GROUP constant is just the naming prefix) Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/55b44603 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/55b44603 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/55b44603 Branch: refs/heads/master Commit: 55b44603e4b0845f6d63f6d5e70953f6fdc1fffe Parents: b1fc16d Author: Aled Sage <[email protected]> Authored: Thu Jun 22 11:55:46 2017 +0100 Committer: Aled Sage <[email protected]> Committed: Thu Jun 22 14:33:32 2017 +0100 ---------------------------------------------------------------------- .../creator/DefaultAzureArmNetworkCreator.java | 31 ++-- .../DefaultAzureArmNetworkCreatorTest.java | 160 +++++++++---------- 2 files changed, 95 insertions(+), 96 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/55b44603/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 179c12c..dc87cfc 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 @@ -46,9 +46,9 @@ public class DefaultAzureArmNetworkCreator { public static final Logger LOG = LoggerFactory.getLogger(DefaultAzureArmNetworkCreator.class); - private static final String DEFAULT_RESOURCE_GROUP = "brooklyn-default-resource-group"; - private static final String DEFAULT_NETWORK_NAME = "brooklyn-default-network"; - private static final String DEFAULT_SUBNET_NAME = "brooklyn-default-subnet"; + private static final String DEFAULT_RESOURCE_GROUP_PREFIX = "brooklyn-default-resource-group"; + private static final String DEFAULT_NETWORK_NAME_PREFIX = "brooklyn-default-network"; + private static final String DEFAULT_SUBNET_NAME_PREFIX = "brooklyn-default-subnet"; private static final String DEFAULT_VNET_ADDRESS_PREFIX = "10.1.0.0/16"; private static final String DEFAULT_SUBNET_ADDRESS_PREFIX = "10.1.0.0/24"; @@ -61,7 +61,7 @@ public class DefaultAzureArmNetworkCreator { public static void createDefaultNetworkAndAddToTemplateOptionsIfRequired(ComputeService computeService, ConfigBag config) { if (!config.get(AZURE_ARM_DEFAULT_NETWORK_ENABLED)) { - LOG.info("azure.arm.default.network.enabled is disabled, not creating default network"); + LOG.debug("azure.arm.default.network.enabled is disabled, not creating default network"); return; } @@ -70,33 +70,33 @@ public class DefaultAzureArmNetworkCreator { //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.info("Network config specified when provisioning Azure machine. Not creating default network"); + LOG.debug("Network config specified when provisioning Azure machine. Not creating default network"); return; } if (templateOptions != null && (templateOptions.containsKey(NETWORK_NAME.getName()) || templateOptions.containsKey("ipOptions"))) { - LOG.info("Network config specified when provisioning Azure machine. Not creating default network"); + LOG.debug("Network config specified when provisioning Azure machine. Not creating default network"); return; } - LOG.info("Network config not specified when provisioning Azure machine. Creating default network if doesn't exist"); - AzureComputeApi api = computeService.getContext().unwrapApi(AzureComputeApi.class); String location = config.get(CLOUD_REGION_ID); - String resourceGroupName = DEFAULT_RESOURCE_GROUP + "-" + location; - String vnetName = DEFAULT_NETWORK_NAME + "-" + location; - String subnetName = DEFAULT_SUBNET_NAME + "-" + location; + String resourceGroupName = DEFAULT_RESOURCE_GROUP_PREFIX + "-" + location; + String vnetName = DEFAULT_NETWORK_NAME_PREFIX + "-" + location; + String subnetName = DEFAULT_SUBNET_NAME_PREFIX + "-" + location; //Check if default already exists Subnet preexistingSubnet = api.getSubnetApi(resourceGroupName, vnetName).get(subnetName); if(preexistingSubnet != null){ - LOG.info("Default Azure network and subnet already created, "+vnetName); + LOG.info("Using pre-existing default Azure network [{}] and subnet [{}] when provisioning machine", + vnetName, subnetName); updateTemplateOptions(config, preexistingSubnet); return; } - LOG.info("Network config not specified when creating Azure location and default network/subnet does not exists. Creating"); + 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); createResourceGroupIfNeeded(api, resourceGroupName, location); @@ -136,12 +136,13 @@ public class DefaultAzureArmNetworkCreator { } private static void createResourceGroupIfNeeded(AzureComputeApi api, String resourceGroup, String location) { - LOG.debug("using resource group [%s]", resourceGroup); ResourceGroup rg = api.getResourceGroupApi().get(resourceGroup); if (rg == null) { - LOG.debug("resource group [%s] does not exist. Creating!", resourceGroup); + LOG.info("Default Azure resource group [{}] does not exist in {}. Creating!", resourceGroup, location); api.getResourceGroupApi().create(resourceGroup, location, ImmutableMap.of("description", "brooklyn default resource group")); + } else { + LOG.debug("Using existing default Azure resource group [{}] in {}", resourceGroup, location); } } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/55b44603/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 832776e..86a5778 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 @@ -22,6 +22,9 @@ import static org.apache.brooklyn.core.location.cloud.CloudLocationConfig.CLOUD_ import static org.apache.brooklyn.location.jclouds.api.JcloudsLocationConfigPublic.NETWORK_NAME; import static org.apache.brooklyn.location.jclouds.api.JcloudsLocationConfigPublic.TEMPLATE_OPTIONS; import static org.apache.brooklyn.location.jclouds.networking.creator.DefaultAzureArmNetworkCreator.AZURE_ARM_DEFAULT_NETWORK_ENABLED; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -30,56 +33,63 @@ import static org.testng.Assert.assertEquals; import java.util.HashMap; import java.util.Map; -import org.mockito.Answers; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Test; - -import com.google.common.collect.ImmutableMap; - +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.features.ResourceGroupApi; import org.jclouds.azurecompute.arm.features.SubnetApi; import org.jclouds.azurecompute.arm.features.VirtualNetworkApi; import org.jclouds.compute.ComputeService; +import org.jclouds.compute.ComputeServiceContext; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; -import org.apache.brooklyn.util.core.config.ConfigBag; +import com.google.common.collect.ImmutableMap; public class DefaultAzureArmNetworkCreatorTest { - @Mock(answer = Answers.RETURNS_DEEP_STUBS) ComputeService computeService; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) AzureComputeApi azureComputeApi; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) ResourceGroupApi resourceGroupApi; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) VirtualNetworkApi virtualNetworkApi; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) SubnetApi subnetApi; + @Mock ComputeService computeService; + @Mock ComputeServiceContext computeServiceContext; + @Mock AzureComputeApi azureComputeApi; + @Mock ResourceGroupApi resourceGroupApi; + @Mock VirtualNetworkApi virtualNetworkApi; + @Mock SubnetApi subnetApi; + @Mock ResourceGroup resourceGroup; @Mock Subnet subnet; - final String TEST_RESOURCE_GROUP = "brooklyn-default-resource-group-test-loc"; - final String TEST_NETWORK_NAME = "brooklyn-default-network-test-loc"; - final String TEST_SUBNET_NAME = "brooklyn-default-subnet-test-loc"; - final String TEST_SUBNET_ID = "/test/resource/id"; final String TEST_LOCATION = "test-loc"; + final String TEST_RESOURCE_GROUP = "brooklyn-default-resource-group-" + TEST_LOCATION; + final String TEST_NETWORK_NAME = "brooklyn-default-network-" + TEST_LOCATION; + final String TEST_SUBNET_NAME = "brooklyn-default-subnet-" + TEST_LOCATION; + final String TEST_SUBNET_ID = "/test/resource/id"; - @BeforeMethod - public void setUp() { + @BeforeMethod(alwaysRun=true) + public void setUp() throws Exception { MockitoAnnotations.initMocks(this); + + // stock mock responses + when(computeService.getContext()).thenReturn(computeServiceContext); + when(computeServiceContext.unwrapApi(AzureComputeApi.class)).thenReturn(azureComputeApi); + when(azureComputeApi.getResourceGroupApi()).thenReturn(resourceGroupApi); + when(azureComputeApi.getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME)).thenReturn(subnetApi); + when(azureComputeApi.getVirtualNetworkApi(TEST_RESOURCE_GROUP)).thenReturn(virtualNetworkApi); + when(subnet.id()).thenReturn(TEST_SUBNET_ID); } @Test - public void testPreExisting() { + public void testPreExisting() throws Exception { //Setup config bag ConfigBag configBag = ConfigBag.newInstance(); configBag.put(CLOUD_REGION_ID, TEST_LOCATION); //Setup mocks - when(computeService.getContext().unwrapApi(AzureComputeApi.class)).thenReturn(azureComputeApi); - when(azureComputeApi.getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME)).thenReturn(subnetApi); when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(subnet); - when(subnet.id()).thenReturn(TEST_SUBNET_ID); + //Test DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); @@ -87,67 +97,70 @@ public class DefaultAzureArmNetworkCreatorTest { //verify verify(subnetApi).get(TEST_SUBNET_NAME); verify(subnet).id(); - verify(azureComputeApi).getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME); + //verify templateOptions updated to include defaults Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); - Map<String, Object> ipOptions = (Map<String, Object>)templateOptions.get("ipOptions"); + Map<?, ?> ipOptions = (Map<?, ?>)templateOptions.get("ipOptions"); assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); assertEquals(ipOptions.get("allocateNewPublicIp"), true); } @Test - public void testVanilla() { + public void testVanillaWhereNoResourceGroup() throws Exception { + runVanilla(ImmutableMap.of()); + } + + @Test + public void testVanillaWhereTemplateOptionsAlreadySpecified() throws Exception { + ImmutableMap<?, ?> additionalConfig = ImmutableMap.of(TEMPLATE_OPTIONS, ImmutableMap.of("unrelated-key", "unrelated-value")); + ConfigBag result = runVanilla(additionalConfig); + Map<String, ?> templateOptions = result.get(TEMPLATE_OPTIONS); + assertEquals(templateOptions.get("unrelated-key"), "unrelated-value"); + } + + protected ConfigBag runVanilla(Map<?, ?> additionalConfig) throws Exception { //Setup config bag ConfigBag configBag = ConfigBag.newInstance(); configBag.put(CLOUD_REGION_ID, TEST_LOCATION); - + configBag.putAll(additionalConfig); + //Setup mocks - when(computeService.getContext().unwrapApi(AzureComputeApi.class)).thenReturn(azureComputeApi); - when(azureComputeApi.getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME)).thenReturn(subnetApi); when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(null).thenReturn(subnet); //null first time, subnet next - when(subnet.id()).thenReturn(TEST_SUBNET_ID); - when(azureComputeApi.getResourceGroupApi()).thenReturn(resourceGroupApi); when(resourceGroupApi.get(TEST_RESOURCE_GROUP)).thenReturn(null); - when(azureComputeApi.getVirtualNetworkApi(TEST_RESOURCE_GROUP)).thenReturn(virtualNetworkApi); - //Test DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); - //verify + //verify calls made verify(subnetApi, times(2)).get(TEST_SUBNET_NAME); verify(subnet).id(); - verify(azureComputeApi, times(2)).getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME); - verify(azureComputeApi, times(2)).getResourceGroupApi(); verify(resourceGroupApi).get(TEST_RESOURCE_GROUP); - verify(azureComputeApi).getVirtualNetworkApi(TEST_RESOURCE_GROUP); + verify(resourceGroupApi).create(eq(TEST_RESOURCE_GROUP), eq(TEST_LOCATION), any()); + + verify(virtualNetworkApi).createOrUpdate(eq(TEST_NETWORK_NAME), eq(TEST_LOCATION), any()); + //verify templateOptions updated to include defaults Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); - Map<String, Object> ipOptions = (Map<String, Object>)templateOptions.get("ipOptions"); + Map<?, ?> ipOptions = (Map<?, ?>)templateOptions.get("ipOptions"); assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); assertEquals(ipOptions.get("allocateNewPublicIp"), true); + + return configBag; } @Test - public void testVanillaWhereTemplateOptionsAlreadySpecified() { + public void testVanillaWhereExistingResourceGroup() throws Exception { //Setup config bag ConfigBag configBag = ConfigBag.newInstance(); configBag.put(CLOUD_REGION_ID, TEST_LOCATION); - configBag.put(TEMPLATE_OPTIONS, ImmutableMap.of("unrelated-key", "unrelated-value")); - + //Setup mocks - when(computeService.getContext().unwrapApi(AzureComputeApi.class)).thenReturn(azureComputeApi); - when(azureComputeApi.getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME)).thenReturn(subnetApi); when(subnetApi.get(TEST_SUBNET_NAME)).thenReturn(null).thenReturn(subnet); //null first time, subnet next - when(subnet.id()).thenReturn(TEST_SUBNET_ID); - when(azureComputeApi.getResourceGroupApi()).thenReturn(resourceGroupApi); - when(resourceGroupApi.get(TEST_RESOURCE_GROUP)).thenReturn(null); - - when(azureComputeApi.getVirtualNetworkApi(TEST_RESOURCE_GROUP)).thenReturn(virtualNetworkApi); + when(resourceGroupApi.get(TEST_RESOURCE_GROUP)).thenReturn(resourceGroup); //Test @@ -156,37 +169,30 @@ public class DefaultAzureArmNetworkCreatorTest { //verify verify(subnetApi, times(2)).get(TEST_SUBNET_NAME); verify(subnet).id(); - verify(azureComputeApi, times(2)).getSubnetApi(TEST_RESOURCE_GROUP, TEST_NETWORK_NAME); - verify(azureComputeApi, times(2)).getResourceGroupApi(); verify(resourceGroupApi).get(TEST_RESOURCE_GROUP); - verify(azureComputeApi).getVirtualNetworkApi(TEST_RESOURCE_GROUP); + verify(resourceGroupApi, never()).create(any(), any(), any()); - Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); - assertEquals(templateOptions.get("unrelated-key"), "unrelated-value"); + verify(virtualNetworkApi).createOrUpdate(eq(TEST_NETWORK_NAME), eq(TEST_LOCATION), any()); - Map<String, Object> ipOptions = (Map<String, Object>)templateOptions.get("ipOptions"); + //verify templateOptions updated to include defaults + Map<String, Object> templateOptions = configBag.get(TEMPLATE_OPTIONS); + Map<?, ?> ipOptions = (Map<?, ?>)templateOptions.get("ipOptions"); assertEquals(ipOptions.get("subnet"), TEST_SUBNET_ID); assertEquals(ipOptions.get("allocateNewPublicIp"), true); } @Test - public void testNetworkInConfig() { + public void testNetworkInConfig() throws Exception { ConfigBag configBag = ConfigBag.newInstance(); configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(NETWORK_NAME, TEST_NETWORK_NAME); - Map<String, Object> configCopy = configBag.getAllConfig(); - - DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); - - //Ensure nothing changed, and no calls were made to the compute service - assertEquals(configCopy, configBag.getAllConfig()); - Mockito.verifyZeroInteractions(computeService); + runAssertingNoIteractions(configBag); } @Test - public void testNetworkInTemplate() { + public void testNetworkInTemplate() throws Exception { HashMap<String, Object> templateOptions = new HashMap<>(); templateOptions.put(NETWORK_NAME.getName(), TEST_NETWORK_NAME); @@ -194,17 +200,11 @@ public class DefaultAzureArmNetworkCreatorTest { configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(TEMPLATE_OPTIONS, templateOptions); - Map<String, Object> configCopy = configBag.getAllConfig(); - - DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); - - //Ensure nothing changed, and no calls were made to the compute service - assertEquals(configCopy, configBag.getAllConfig()); - Mockito.verifyZeroInteractions(computeService); + runAssertingNoIteractions(configBag); } @Test - public void testIpOptionsInTemplate() { + public void testIpOptionsInTemplate() throws Exception { HashMap<String, Object> templateOptions = new HashMap<>(); templateOptions.put("ipOptions", TEST_NETWORK_NAME); @@ -212,21 +212,19 @@ public class DefaultAzureArmNetworkCreatorTest { configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(TEMPLATE_OPTIONS, templateOptions); - Map<String, Object> configCopy = configBag.getAllConfig(); - - DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag); - - //Ensure nothing changed, and no calls were made to the compute service - assertEquals(configCopy, configBag.getAllConfig()); - Mockito.verifyZeroInteractions(computeService); + runAssertingNoIteractions(configBag); } @Test - public void testConfigDisabled() { + public void testConfigDisabled() throws Exception { ConfigBag configBag = ConfigBag.newInstance(); configBag.put(CLOUD_REGION_ID, TEST_LOCATION); configBag.put(AZURE_ARM_DEFAULT_NETWORK_ENABLED, false); + runAssertingNoIteractions(configBag); + } + + protected void runAssertingNoIteractions(ConfigBag configBag) throws Exception { Map<String, Object> configCopy = configBag.getAllConfig(); DefaultAzureArmNetworkCreator.createDefaultNetworkAndAddToTemplateOptionsIfRequired(computeService, configBag);
