You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2017/06/23 11:22:23 UTC
[1/2] brooklyn-server git commit: DefaultAzureArmNetworkCreator
improvements
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 <al...@gmail.com>
Authored: Thu Jun 22 11:55:46 2017 +0100
Committer: Aled Sage <al...@gmail.com>
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);
[2/2] brooklyn-server git commit: This closes #742
Posted by al...@apache.org.
This closes #742
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/169b55e4
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/169b55e4
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/169b55e4
Branch: refs/heads/master
Commit: 169b55e4195f73f1607fea72e384195699edaafb
Parents: 733fe77 55b4460
Author: Aled Sage <al...@gmail.com>
Authored: Fri Jun 23 12:22:09 2017 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Jun 23 12:22:09 2017 +0100
----------------------------------------------------------------------
.../creator/DefaultAzureArmNetworkCreator.java | 31 ++--
.../DefaultAzureArmNetworkCreatorTest.java | 160 +++++++++----------
2 files changed, 95 insertions(+), 96 deletions(-)
----------------------------------------------------------------------