You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Rita Zhang <no...@github.com> on 2016/05/23 18:19:49 UTC
[jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
@nacx @andreaturli
This PR is for DeploymentApi, OSImageApi, and VMSizeApi, part of the foundations needed for interacting with Azure Resource Manager (ARM) Rest APIs.
FYI
@jtjk @isalento @jmspring
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/273
-- Commit Summary --
* JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi
-- File Changes --
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeApi.java (27)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/options/AzureComputeArmTemplateOptions.java (419)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/ComputeNode.java (36)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Deployment.java (277)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/DeploymentBody.java (42)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/DeploymentProperties.java (31)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/DeploymentTemplate.java (103)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Offer.java (51)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Publisher.java (52)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/ResourceDefinition.java (103)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/SKU.java (51)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/StorageService.java (48)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/VMDeployment.java (29)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/VMSize.java (67)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Version.java (49)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/DeploymentApi.java (110)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/NetworkInterfaceCardApi.java (8)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/OSImageApi.java (89)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/VMSizeApi.java (47)
M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/functions/ParseJobStatus.java (3)
A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/DeploymentTemplateBuilder.java (356)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceContextLiveTest.java (292)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/DeploymentApiLiveTest.java (127)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/DeploymentApiMockTest.java (105)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/DeploymentTemplateBuilderTest.java (219)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/JobApiMockTest.java (12)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkInterfaceCardApiLiveTest.java (34)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkInterfaceCardApiMockTest.java (11)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/OSImageApiLiveTest.java (77)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/OSImageApiMockTest.java (80)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/TemplateToDeploymentTemplateLiveTest.java (196)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VMSizeApiLiveTest.java (40)
A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/VMSizeApiMockTest.java (55)
M azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java (4)
A azurecompute-arm/src/test/resources/createdeploymentaccepted.json (33)
A azurecompute-arm/src/test/resources/createdeploymentsucceeded.json (36)
A azurecompute-arm/src/test/resources/listdeployments.json (99)
A azurecompute-arm/src/test/resources/offers.json (7)
A azurecompute-arm/src/test/resources/publishers.json (12)
A azurecompute-arm/src/test/resources/skus.json (12)
A azurecompute-arm/src/test/resources/versions.json (12)
A azurecompute-arm/src/test/resources/vmsizes.json (28)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/273.patch
https://github.com/jclouds/jclouds-labs/pull/273.diff
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx Ready for another round of review. The only outstanding question is how to deal with `$schema`.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222559088
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -0,0 +1,292 @@
> +/*
Remove the class from this PR, as there is no compute service yet.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64480479
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + assertTrue(state == ProvisioningState.SUCCEEDED);
> + }
> +
> +
> + @Test(groups = "live", dependsOnMethods = "testCreate")
> + public void testGetDeployment() {
> + Deployment deployment = api().get(deploymentName);
> + assertNotNull(deployment);
> + ProvisioningState state = ProvisioningState.fromValue(deployment.properties().provisioningState());
> + assertTrue(state == ProvisioningState.SUCCEEDED);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreate")
> + public void testListDeployments() {
> + List<Deployment> deployments = api().list();
> + assertTrue(deployments.size() > 0 && deployments.get(0).name().equals(deploymentName));
If the live tests are run in an account that already has data, is it correct to expect the one used in the tests to be returned in the first position?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65270520
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
> +
> + Deployment deployment = api().create(deploymentName, properties);
> + assertNotNull(deployment);
> +
> + ProvisioningState state = ProvisioningState.fromString(deployment.properties().provisioningState());
> + int testTime = 0;
> + while (testTime < maxTestDuration) {
> + if ((state == ProvisioningState.SUCCEEDED) ||
Use the jclouds style retries, and consider providing a generic predicate that could be injected. This was already commented on past pull requests.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481183
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
> I don't think anyone would really use it directly
@nacx Would users ever call the DeploymentApi directly? As @jmspring mentioned, if users always leverage jclouds templates to provision everything, then the APIs should reflect the raw APIs. WDYT?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64762789
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -30,6 +30,7 @@
> DONE,
> IN_PROGRESS,
> FAILED,
> + NO_CONTENT,
What does this status mean?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481730
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/listdeployments.json"));
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() > 0);
> + }
> +
> + @Test
> + public void testListDeploymentEmpty() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + server.enqueue(new MockResponse().setResponseCode(404));
> +
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() == 0);
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65290816
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ilkka Salento <no...@github.com>.
> + .apiVersion("2015-06-15")
> + .properties(
> + StorageServiceProperties.builder()
> + .accountType(StorageService.AccountType.Standard_LRS)
> + .build()
> + )
> + .build();
> +
> + resources.add(storageAccount);
> + }
> +
> + private void addVirtualNetworkResource() {
> + String virtualNetworkName = group + "virtualnetwork";
> + String vnAddresSpacePrefix = "10.0.0.0/16";
> + String subnetName = group + "subnet";
> + String subnetAddressPrefix = "10.0.0.0/24";
Fair point. Maybe not hardcoded, but we need to have default values for vn / subnet CIDR blocks as Azure does not assign default values internally. Basically we need to set some values, unless user overrides these by using Azure specific template options (which is not yet implemented).
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64893442
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + @Override
> + public boolean apply(URI uri) {
> + System.out.println(uri.toString());
> + System.out.println(api.getJobApi().jobStatus(uri).name());
> + return ParseJobStatus.JobStatus.NO_CONTENT == api.getJobApi().jobStatus(uri);
> + }
> + }, 60 * maxTestDuration * 1000).apply(uri);
> + assertTrue(jobDone, "delete operation did not complete in the configured timeout");
> + }
> + }
> + }
> +
> + private DeploymentApi api() {
> + return api.getDeploymentApi(resourceName);
> + }
> +}
Add a test (or call it in an existing one) that exercises the `DeploymentApi.validate` method
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65270865
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> + assertTrue(state == ProvisioningState.SUCCEEDED);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreate")
> + public void testGetDeployment() {
> + Deployment deployment = api().get(deploymentName);
> + assertNotNull(deployment);
> + ProvisioningState state = ProvisioningState.fromString(deployment.properties().provisioningState());
> + assertTrue(state == ProvisioningState.SUCCEEDED);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testCreate")
> + public void testListDeployments() {
> + List<Deployment> deployments = api().list();
> + assertTrue(deployments.size() > 0);
Verify that the created deployment exists.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481299
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
Hmmm I mean, in the live test, the actual body passed to the API method is the result of the `getPutBody ` method, but this method is only defined in the tests (which means that users will have to manually generate the json).
When I ask about the "abstracting the raw json template with a builder", I mean a builder that already produces the right body to be sent in the request (the `getPutBody`). I imagine something like `TempalteBuilder.foo().bar().mode(mode).parameters(parameters).build()` or `TempalteBuilder.foo().bar().build(mode, parameters)` that already produces what the `getPutBody` does in the tests, so users can actually forget about manually building json strings.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65270248
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
> But the user experience, in this case is worse: Users have to build on their own that json. They can use the builders, but they have to manually serialize the template into a json, just because our DeploymentApi methods expect the template to be a json String.
I agree the user experience is not great today, but this is how users are interfacing with the Azure ARM template deployment process today and this is the intended behavior of the original Deployment API. From the official API doc, the template "Specifies the JSON definition for the template. You use this element when you want to pass the template syntax directly in the request rather than link to an existing template."
If users can call the DeploymentApi directly, then in this case, users will expect to pass in a raw json template to the api much like how they interface with the original api anywhere whether it's thru a REST api call, or thru the Azure CLI, or thru an SDK. [Here is the reference](https://azure.microsoft.com/en-us/documentation/articles/resource-group-authoring-templates/)
Given that this is the design of this particular Deployment API and the Azure ARM template deployment process, we should probably follow the intent of the original api and ensure we give users the same experience as they expect it elsewhere.
However if users use this thru the jclouds api, then we want to abstract all this raw template business by providing developers with the `DeploymentTemplateBuilder` so they do not have to deal with creating the raw json but instead pass in the parameters and we will build the json template for them.
@nacx WDYT? The current implementation allows users to use the DeploymentApi directly with the raw json template as intended by the original api, but it also supports abstracting the raw json template with a builder if users use jclouds apis.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65021912
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
_bump_ @nacx I have split the previous PR #267 as two. This one only has the APIs. Thanks!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-221307094
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
> + protected String storageAccountName;
> + protected String storageAccountType;
You are right. I only see reference of `authorizePublicKey`, `inboundPorts`, `getPublicKey`, `getLoginUser`, `getLoginPassword`. @jtjk can you please take a look?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64491110
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/createdeploymentsucceeded.json"));
> + deployment = deploymentApi.create(deploymentName, properties);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.SUCCEEDED);
> + }
> +
> + @Test
> + public void testGetDeployment() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/createdeploymentsucceeded.json"));
> + Deployment deployment = deploymentApi.get(deploymentName);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.SUCCEEDED);
Verify the recorded request
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65271128
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +@AutoValue
> +public abstract class DeploymentTemplate {
> +
> + //Empty placeholders as we want to generate the empty JSON object
> + @AutoValue
> + public abstract static class Parameters {
> + public static Parameters create() {
> + return new AutoValue_DeploymentTemplate_Parameters();
> + }
> + }
> +
> + public abstract String $schema();
`$schema` is actually a json keyword. It is used here to specify the schema of the template. Check out this [reference](https://msdn.microsoft.com/en-us/library/azure/dn790564.aspx).
```
"properties": {
"template": {
"$schema": "http://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
...
```
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64495870
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> +@Path("/resourcegroups/{resourcegroup}/providers/microsoft.resources/deployments")
> +@QueryParams(keys = "api-version", values = "2016-02-01")
> +@RequestFilters(OAuthFilter.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +public interface DeploymentApi {
> +
> + /**
> + * The Create Template Deployment operation starts the process of an ARM Template deployment.
> + * It then returns a Deployment object.
> + */
> + @Named("deployment:create")
> + @Path("/{deploymentname}")
> + @Payload("{properties}")
> + @PUT
> + @Produces(MediaType.APPLICATION_JSON)
> + @Fallback(NullOnNotFoundOr404.class)
ok.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64496180
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + .apiVersion("2015-06-15")
> + .properties(
> + StorageServiceProperties.builder()
> + .accountType(StorageService.AccountType.Standard_LRS)
> + .build()
> + )
> + .build();
> +
> + resources.add(storageAccount);
> + }
> +
> + private void addVirtualNetworkResource() {
> + String virtualNetworkName = group + "virtualnetwork";
> + String vnAddresSpacePrefix = "10.0.0.0/16";
> + String subnetName = group + "subnet";
> + String subnetAddressPrefix = "10.0.0.0/24";
@jtjk @isalento please take a look at this.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64496895
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/listdeployments.json"));
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() > 0);
> + }
> +
> + @Test
> + public void testListDeploymentEmpty() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + server.enqueue(new MockResponse().setResponseCode(404));
> +
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() == 0);
Verify the recorded request
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65271227
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + variables.put("osDiskName", osDiskName);
> +
> + OSDisk osDisk = OSDisk.builder()
> + .name("[variables('osDiskName')]")
> + .vhd(
> + VHD.create("[concat('http://',variables('storageAccountName'),'.blob.core.windows.net/',variables('storageAccountContainerName'),'/',variables('osDiskName'),'.vhd')]")
> + )
> + .caching("ReadWrite")
> + .createOption("FromImage")
> + .build();
> +
> +
> + //Create Data Disk(s) and add to list
> + final String dataDiskName = name + "datadisk";
> + variables.put("dataDiskName", dataDiskName);
> + final String dataDiskSize = "100";
>Should we just drop data disk creation until Azure template option has support for it or create a singe data disk using 1023 as default size?
+1
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64720903
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @ritazh! Initial review done. Apart from the inline comments, please apply the jclouds formatting to all the classes.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-221415229
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + * Get Deployment Information returns information about the specified deployment.
> + */
> + @Named("deployment:get")
> + @Path("/{deploymentname}")
> + @GET
> + @Fallback(NullOnNotFoundOr404.class)
> + Deployment get(@PathParam("deploymentname") String deploymentname);
> +
> + /**
> + * Validate Deployment validates deployment template before deployment
> + */
> + @Named("deployment:validate")
> + @Path("/{deploymentname}/validate")
> + @Payload("{properties}")
> + @POST
> + @Fallback(NullOnNotFoundOr404.class)
Remove fallback
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64479286
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment accepted
> + server.enqueue(jsonResponse("/createdeploymentaccepted.json"));
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
> + Deployment deployment = deploymentApi.create(deploymentName, properties);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.ACCEPTED);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/createdeploymentsucceeded.json"));
> + deployment = deploymentApi.create(deploymentName, properties);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.SUCCEEDED);
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65290789
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
Ok, let's keep the current API signature then. Thanks for taking your tine to discuss and dissgree! :)
>WDYT? The current implementation allows users to use the DeploymentApi directly with the raw json template as intended by the original api, but it also supports abstracting the raw json template with a builder if users use jclouds apis
That sounds good. Does the builder already generate the json?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65033690
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/listdeployments.json"));
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() > 0);
> + }
> +
> + @Test
> + public void testNullGetDeployment() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + server.enqueue(new MockResponse().setResponseCode(404));
> + assertNull(deploymentApi.get("randomName"));
> +
> + assertSent(server, "GET", "/subscriptions/" + subscriptionId + "/resourcegroups/" + resourceGroup +
> + "/providers/microsoft.resources/deployments/randomName?api-version=2016-02-01");
> + }
Add all missing tests: fallbacks, delete...
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481490
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Just two minors left. Mind squashing the commits directly while addressing them? Let's get this merged!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-223346347
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +@Path("/resourcegroups/{resourcegroup}/providers/microsoft.resources/deployments")
> +@QueryParams(keys = "api-version", values = "2016-02-01")
> +@RequestFilters(OAuthFilter.class)
> +@Consumes(MediaType.APPLICATION_JSON)
> +public interface DeploymentApi {
> +
> + /**
> + * The Create Template Deployment operation starts the process of an ARM Template deployment.
> + * It then returns a Deployment object.
> + */
> + @Named("deployment:create")
> + @Path("/{deploymentname}")
> + @Payload("{properties}")
> + @PUT
> + @Produces(MediaType.APPLICATION_JSON)
> + @Fallback(NullOnNotFoundOr404.class)
Don't use 404 fallbacks in POST/PUT
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64477297
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
yes it does.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65096117
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Do you have a link to just the last commit? I'll be able to review it quickly. Otherwise it will take a bit longer, as I need to allocate some time to check the whole PR taking special care to the changed bits :)
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222910812
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
>Ready for another round of review. The only outstanding question is how to deal with $schema.
Done reviewing. Regarding the `$schema` thing, AFAIK it works both ways. I've just isolated the case and put a very simple example to prove the behavior:
```java
@AutoValue
public abstract class DeploymentTemplate {
public abstract String schema();
public abstract String contentVersion();
@SerializedNames({"$schema", "contentVersion"})
public static DeploymentTemplate create(final String schema, final String contentVersion) {
return new AutoValue_DeploymentTemplate(schema, contentVersion);
}
public static void main(final String[] args) throws Exception {
Properties props = new Properties();
props.setProperty("oauth.endpoint", "https://login.microsoftonline.com/<subscriptionId>/oauth2/token");
// Just build the context to have access to the jclouds Utils class
ApiContext<AzureComputeApi> ctx = ContextBuilder.newBuilder("azurecompute-arm")
.credentials("<identity>", "<credential>")
.overrides(props)
.build();
// Get the json instance with the Gson configuration used by jclouds to serialize/deserialize
Json json = ctx.utils().json();
// Verify serialization
DeploymentTemplate t = DeploymentTemplate.create("http://localhost", "1.0.0");
String toJson = json.toJson(t);
assert toJson.equals("{\"$schema\":\"http://localhost\",\"contentVersion\":\"1.0.0\"}");
// Verify deserialization
DeploymentTemplate fromString = json.fromJson("{\"$schema\":\"http://localhost\",\"contentVersion\":\"1.0.0\"}", DeploymentTemplate.class);
assert fromString.schema().equals("http://localhost");
}
}
```
@isalento @jmspring Could you have a look?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222838147
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/createdeploymentsucceeded.json"));
> + deployment = deploymentApi.create(deploymentName, properties);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.SUCCEEDED);
> + }
> +
> + @Test
> + public void testGetDeployment() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/createdeploymentsucceeded.json"));
> + Deployment deployment = deploymentApi.get(deploymentName);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.SUCCEEDED);
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65290799
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + protected String reservedIPName;
> +
> + @Override
> + public AzureComputeArmTemplateOptions clone() {
> + final AzureComputeArmTemplateOptions options = new AzureComputeArmTemplateOptions();
> + copyTo(options);
> + return options;
> + }
> +
> + @Override
> + public void copyTo(final TemplateOptions to) {
> + super.copyTo(to);
> + if (to instanceof AzureComputeArmTemplateOptions) {
> + final AzureComputeArmTemplateOptions eTo = AzureComputeArmTemplateOptions.class.cast(to);
> + eTo.virtualNetworkName(virtualNetworkName);
> + if (!subnetNames.isEmpty()) {
A clone operation should be consistent. Why not cloning an empty list? If not, the result is not a clone, but an aggregate.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64475077
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Janne Koskinen <no...@github.com>.
> + DeploymentBody body = DeploymentBody.create(template, DEPLOYMENT_MODE, DeploymentTemplate.Parameters.create());
> +
> + return body;
> + }
> +
> +
> + private void addStorageResource() {
> + String storageAccountName = name.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +
> + variables.put("storageAccountName", storageAccountName);
> +
> + ResourceDefinition storageAccount = ResourceDefinition.builder()
> + .name("[variables('storageAccountName')]")
> + .type("Microsoft.Storage/storageAccounts")
> + .location(location)
> + .apiVersion("2015-06-15")
This is little bit tricky. If we have constant like STORAGE_API_VERSION and then we update it and update also StorageAccountAPI, we may face problems with this part of code. Maybe we should have that constant but in that case we should be very careful when updating the version to remember to update all code that is affected.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64696785
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + List<Deployment> deployments = api().list();
> + for (Deployment d : deployments) {
> + if (d.name().contains("jcdep")) {
> + URI uri = api().delete(d.name());
> + if (uri != null){
> + assertTrue(uri.toString().contains("api-version"));
> + assertTrue(uri.toString().contains("operationresults"));
> + }
> + }
> + }
> + }
> +
> + private DeploymentApi api() {
> + return api.getDeploymentApi(resourceGroup);
> + }
> +}
Also test the `validate` method
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481414
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> + }
> +
> + @AutoValue
> + public abstract static class Provider {
> + @Nullable
> + public abstract String id();
> +
> + @Nullable
> + public abstract String namespace();
> +
> + @Nullable
> + public abstract String registrationState();
> +
> + @Nullable
> + public abstract List<ProviderResourceType> resourceTypes();
Same here about nullable collections.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64476426
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + List<SKU> skus = api.getOSImageApi(location).listSKUs(publisher, offer);
> +
> + assertEquals(size(skus), 2);
> +
> + assertSent(server, "GET", requestUrl + "/" + publisher + "/artifacttypes/vmimage/offers/" + offer + "/skus" + apiversion);
> + }
> +
> + public void testVersions() throws InterruptedException {
> + server.enqueue(jsonResponse("/versions.json"));
> +
> + List<Version> versions = api.getOSImageApi(location).listVersions(publisher, offer, sku);
> +
> + assertEquals(size(versions), 2);
> +
> + assertSent(server, "GET", requestUrl + "/" + publisher + "/artifacttypes/vmimage/offers/" + offer + "/skus/" + sku + "/versions" + apiversion);
> + }
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65107766
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + .apiVersion("2015-06-15")
> + .properties(
> + StorageServiceProperties.builder()
> + .accountType(StorageService.AccountType.Standard_LRS)
> + .build()
> + )
> + .build();
> +
> + resources.add(storageAccount);
> + }
> +
> + private void addVirtualNetworkResource() {
> + String virtualNetworkName = group + "virtualnetwork";
> + String vnAddresSpacePrefix = "10.0.0.0/16";
> + String subnetName = group + "subnet";
> + String subnetAddressPrefix = "10.0.0.0/24";
Is this range meant to be hardcoded?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64480165
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + *
> + * 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.azurecompute.arm.domain;
> +
> +public class ComputeNode {
> + public enum Status {
> + GOOD,
> + BAD,
> + UNRECOGNIZED;
> +
> + public static Status fromString(final String text) {
Convention is to name the method `fromValue`, to take benefit from the [EnumTypeAdapterThatReturnsFromValue](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/internal/EnumTypeAdapterThatReturnsFromValue.java) if this has to be deserialised. Also apply this to all the enums.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64475808
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + variables.put("osDiskName", osDiskName);
> +
> + OSDisk osDisk = OSDisk.builder()
> + .name("[variables('osDiskName')]")
> + .vhd(
> + VHD.create("[concat('http://',variables('storageAccountName'),'.blob.core.windows.net/',variables('storageAccountContainerName'),'/',variables('osDiskName'),'.vhd')]")
> + )
> + .caching("ReadWrite")
> + .createOption("FromImage")
> + .build();
> +
> +
> + //Create Data Disk(s) and add to list
> + final String dataDiskName = name + "datadisk";
> + variables.put("dataDiskName", dataDiskName);
> + final String dataDiskSize = "100";
Is this meant to be hardcoded? Would this be extracted from the hardware profile or a template option?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64480135
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Janne Koskinen <no...@github.com>.
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
> + protected String storageAccountName;
> + protected String storageAccountType;
Yep... I commented to first comment already. So we can remove all unused fields
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64696560
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx Sorry about that. Next time I will wait to squash.
Here are the changes in the last commit:
Changes in `BaseAzureComputeApiLiveTest.java` to create context:
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java#L145-L155
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/internal/BaseAzureComputeApiLiveTest.java#L72-L91
Changed all the schema fields in `DeplpoymentTemplate.java`:
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/DeploymentTemplate.java
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/DeploymentTemplateBuilder.java#L109
Calling `getContext()` in `TemplateToDeploymentTemplateLiveTest.java`:
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/TemplateToDeploymentTemplateLiveTest.java#L58
Using `org.jclouds.json.Json` instead of `Gson` for serialization:
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/TemplateToDeploymentTemplateLiveTest.java#L72
https://github.com/ritazh/jclouds-labs/blob/ed40df18648ddd8602ee3167bf491a05cb14df72/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/TemplateToDeploymentTemplateLiveTest.java#L95
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222998958
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ilkka Salento <no...@github.com>.
> + variables.put("osDiskName", osDiskName);
> +
> + OSDisk osDisk = OSDisk.builder()
> + .name("[variables('osDiskName')]")
> + .vhd(
> + VHD.create("[concat('http://',variables('storageAccountName'),'.blob.core.windows.net/',variables('storageAccountContainerName'),'/',variables('osDiskName'),'.vhd')]")
> + )
> + .caching("ReadWrite")
> + .createOption("FromImage")
> + .build();
> +
> +
> + //Create Data Disk(s) and add to list
> + final String dataDiskName = name + "datadisk";
> + variables.put("dataDiskName", dataDiskName);
> + final String dataDiskSize = "100";
Hi, Yes eventually data disk information (number of disks and their sizes) should be set using template option. In Azure the number of supported data disks depends on hardware in use, but the max size for singe disk is 1023 GB. Should we just drop data disk creation until Azure template option has support for it or create a singe data disk using 1023 as default size?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64720067
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> +
> + Deployment deployment = deploymentApi.get(deploymentName);
> + assertNull(deployment);
> +
> + assertSent(server, "GET", "/subscriptions/" + subscriptionId + "/resourcegroups/" + resourceGroup +
> + "/providers/microsoft.resources/deployments/" + deploymentName + "?api-version=2016-02-01");
> + }
> +
> + @Test
> + public void testListDeployment() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/listdeployments.json"));
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() > 0);
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65290808
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + DeploymentBody deploymentTemplateBody = templateBuilder.getDeploymentTemplate();
> + DeploymentProperties properties = DeploymentProperties.create(deploymentTemplateBody);
> +
> + Gson gson = new GsonBuilder().disableHtmlEscaping().create();
> + org.jclouds.json.Json json = new GsonWrapper(gson);
> +
> + String deploymentTemplate = json.toJson(properties);
> + deploymentTemplate = UrlEscapers.urlFormParameterEscaper().escape(deploymentTemplate);
> +
> + //creates an actual VM using deployment template
> + Deployment deployment = api().create(deploymentName, deploymentTemplate);
> +
> + Deployment.ProvisioningState state = Deployment.ProvisioningState.fromString(deployment.properties().provisioningState());
> + int testTime = 0;
> + while (testTime < maxTestDuration) {
Same comment about polling and reusable predicates bound to the Guice injector. This predicates are used in tests and potentially by the compute service, so please, make them injectable so they can be shared, and the polling parameters configured.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64482551
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment accepted
> + server.enqueue(jsonResponse("/createdeploymentaccepted.json"));
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
> + Deployment deployment = deploymentApi.create(deploymentName, properties);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.ACCEPTED);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/createdeploymentsucceeded.json"));
> + deployment = deploymentApi.create(deploymentName, properties);
> + assertTrue(deployment != null);
> + assertEquals(ProvisioningState.fromValue(deployment.properties().provisioningState()), ProvisioningState.SUCCEEDED);
Verify the recorded requests
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65271102
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx That was exactly it! Thank you so much for your help!!! Got all the issues resolved now. Also squashed all the commits as one. Ready for another round of review.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222907079
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + DeploymentBody body = DeploymentBody.create(template, DEPLOYMENT_MODE, DeploymentTemplate.Parameters.create());
> +
> + return body;
> + }
> +
> +
> + private void addStorageResource() {
> + String storageAccountName = name.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +
> + variables.put("storageAccountName", storageAccountName);
> +
> + ResourceDefinition storageAccount = ResourceDefinition.builder()
> + .name("[variables('storageAccountName')]")
> + .type("Microsoft.Storage/storageAccounts")
> + .location(location)
> + .apiVersion("2015-06-15")
Added `public static final String STORAGE_API_VERSION = "2015-06-15";` to `AzureComputeProperties` and referencing it in both `StorageAccountApi` and `DeploymentTemplateBuilder`.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65100178
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
I updated `DeploymentApiLiveTest` to use `DeploymentTemplateBuilder` now so it’s all hooked up.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65290761
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx Thank you for taking a look at this. It's probably something really obvious and I just missed it somehow.
These are the implementation files:
https://gist.github.com/ritazh/0171333aaa69839763e88c50d59d28ed
https://gist.github.com/ritazh/1df20e75be3f604f4538b4fae217888c#file-deploymenttemplatebuilder-java-L109
This is the failing test:
https://gist.github.com/ritazh/3232ae0cbaa8cd2c7d932023cd04f2e9#file-templatetodeploymenttemplatelivetest-java-L127
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222851546
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Jim Spring <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
@nacx - The plan was to basically reflect the raw API itself, thus the three JSON inputs. For generating VMs, users should use the higher level functions, but the process within the actual code will essentially be declare as a JClouds template, translate from that representation to an ARM template (JSON) then use the DeploymentAPI to deploy said template.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64677565
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.QueryParams;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.SelectJson;
> +
> +import javax.inject.Named;
> +import javax.ws.rs.Consumes;
> +import javax.ws.rs.GET;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.Produces;
> +import javax.ws.rs.core.MediaType;
> +import java.util.List;
> +
> +@Path("/providers/Microsoft.Compute/locations/{location}/vmSizes")
> +@QueryParams(keys = "api-version", values = "2015-06-15")
> +@RequestFilters(OAuthFilter.class)
> +@Produces(MediaType.APPLICATION_JSON)
There are no methods here that produce a body
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64479783
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +@AutoValue
> +public abstract class DeploymentTemplate {
> +
> + //Empty placeholders as we want to generate the empty JSON object
> + @AutoValue
> + public abstract static class Parameters {
> + public static Parameters create() {
> + return new AutoValue_DeploymentTemplate_Parameters();
> + }
> + }
> +
> + public abstract String $schema();
@nacx We are using `org.jclouds.json.SerializedNames` and I do not think it supports mapping `schema` to `$schema` here. Do you suggest we use another implementation of `SerializedNames` or were you thinking of something else?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65206156
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.QueryParams;
> +import org.jclouds.rest.annotations.RequestFilters;
> +
> +/**
> + * The Azure Resource Management API includes operations for managing the OS images in your subscription.
> + */
> +@Path("/providers/Microsoft.Compute/locations/{location}")
> +@RequestFilters(OAuthFilter.class)
> +@QueryParams(keys = "api-version", values = "2015-06-15")
> +@Consumes(APPLICATION_JSON)
> +public interface OSImageApi {
> +
> + /**
> + * List Publishers in location
> + */
> + @Named("ListPublishers")
Follow the same naming pattern than in the other apis.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64479558
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +/**
> + * The Azure Resource Management API includes operations for managing the OS images in your subscription.
> + */
> +@Path("/providers/Microsoft.Compute/locations/{location}")
> +@RequestFilters(OAuthFilter.class)
> +@QueryParams(keys = "api-version", values = "2015-06-15")
> +@Consumes(APPLICATION_JSON)
> +public interface OSImageApi {
> +
> + /**
> + * List Publishers in location
> + */
> + @Named("ListPublishers")
> + @GET
> + @Produces(APPLICATION_JSON)
GET methods don't produce a body
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64479678
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx I guess the issue with `$schema` (after testing it) is not with `SerializedNames`, but with builders. When I use the builder to pass in the `schema` parameter
```
DeploymentTemplate template = DeploymentTemplate.builder()
.schema("https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#")
```
I get this error:
```
java.lang.IllegalArgumentException: {"error":{"code":"InvalidRequestContent","message":"The request content was invalid and could not be deserialized: 'Could not find member 'schema' on object of type 'Template'. Path 'properties.template.schema', line 1, position 36.'."}}
```
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222842327
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Hmmmm The builders shouldn't affect, as the serialization will happen on the actual object (not the builder) and deserialization will use the static factory method. The same test case using the builder is working:
```java
@AutoValue
public abstract class DeploymentTemplate {
public abstract String schema();
public abstract String contentVersion();
@SerializedNames({"$schema", "contentVersion"})
public static DeploymentTemplate create(final String schema, final String contentVersion) {
return DeploymentTemplate.builder().schema(schema).contentVersion(contentVersion).build();
}
public static Builder builder() {
return new AutoValue_DeploymentTemplate.Builder();
}
@AutoValue.Builder
public abstract static class Builder {
public abstract Builder schema(String schema);
public abstract Builder contentVersion(String type);
public abstract DeploymentTemplate build();
}
public static void main(final String[] args) throws Exception {
Properties props = new Properties();
props.setProperty("oauth.endpoint", "https://login.microsoftonline.com/<subscriptionId>/oauth2/token");
ApiContext<AzureComputeApi> ctx = ContextBuilder.newBuilder("azurecompute-arm")
.credentials("<identity>", "<credential>")
.overrides(props)
.build();
Json json = ctx.utils().json();
DeploymentTemplate t = DeploymentTemplate.builder()
.schema("http://localhost")
.contentVersion("1.0.0")
.build();
String toJson = json.toJson(t);
assert toJson.equals("{\"$schema\":\"http://localhost\",\"contentVersion\":\"1.0.0\"}");
DeploymentTemplate fromString = json.fromJson("{\"$schema\":\"http://localhost\",\"contentVersion\":\"1.0.0\"}",
DeploymentTemplate.class);
assert fromString.schema().equals("http://localhost");
}
}
```
Do you have an isolated test case I can use to reproduce the issue, in case I'm not getting what do you mean? (A link to a gist would be perfect).
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222849149
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + DeploymentBody body = DeploymentBody.create(template, DEPLOYMENT_MODE, DeploymentTemplate.Parameters.create());
> +
> + return body;
> + }
> +
> +
> + private void addStorageResource() {
> + String storageAccountName = name.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +
> + variables.put("storageAccountName", storageAccountName);
> +
> + ResourceDefinition storageAccount = ResourceDefinition.builder()
> + .name("[variables('storageAccountName')]")
> + .type("Microsoft.Storage/storageAccounts")
> + .location(location)
> + .apiVersion("2015-06-15")
@jtjk @isalento please take a look at this.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64496939
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Test(groups = "live", dependsOnMethods = {"testGetDeployment", "testListDeployments"}, alwaysRun = true)
> + public void testDelete() throws Exception {
> + List<Deployment> deployments = api().list();
> + for (Deployment d : deployments) {
> + if (d.name().contains("jcdep")) {
> + URI uri = api().delete(d.name());
> + assertNotNull(uri);
> + assertTrue(uri.toString().contains("api-version"));
> + assertTrue(uri.toString().contains("operationresults"));
> +
> + boolean jobDone = Predicates2.retry(new Predicate<URI>() {
> + @Override
> + public boolean apply(URI uri) {
> + System.out.println(uri.toString());
> + System.out.println(api.getJobApi().jobStatus(uri).name());
Remove this or use the anonymous logger
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65270745
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
> + protected String storageAccountName;
> + protected String storageAccountType;
What are these options used for?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64474828
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
>The plan was to basically reflect the raw API itself, thus the three JSON inputs
Well, the three inputs only exist in the test. The method int he API just gets one raw JSON parameter, which turns to be the request body. Every single method in the raw API accepts a json payload. In this case, it happens to be a deployment template, but all other method in the API accept raw jsons too, be it a network, a disk or a deployment template. Why should we be providing high level and object oriented methods for some methods, and some other methods that require already serialized json? I understand that the template is a json object, but I think the user experience is not good here: users need to know serialization details. Given that we already have builder objects to be the template "users should be serializing", couldn't we let the api method accept what that builder produces, and let jclouds serialize it? Looks much less error-prone and a better user experience?
>Would users ever call the DeploymentApi directly
jclouds should allow users to call the DeploymentApi directly, yes. And they'll do, eventually, when they want to tweak a deployment in a way the portable abstraction does not let them to do so.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65012066
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Janne Koskinen <no...@github.com>.
> + * import {@code AzureComputeArmTemplateOptions.*} and invoke a static creation method followed by an instance mutator (if
> + * needed):
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
You are correct. We should remove all obsolete fields from AzureComputeArmTemplateOptions. I guess some of the fields were left there as they existed in original Azure template options and we thought that there might be use for them later. In this phase it would be good to remove all unused fields.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64696316
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> + }
> +
> + @AutoValue
> + public abstract static class ProviderResourceType {
> + @Nullable
> + public abstract String resourceType();
> +
> + @Nullable
> + public abstract List<String> locations();
> +
> + @Nullable
> + public abstract List<String> apiVersions();
> +
> + @Nullable
> + public abstract Map<String, JsonBall> properties();
Is there a need for collections to be nullable? Is this the same case than the `tags` properties?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64476244
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
One last thing I think would be very helpful: with the current approach users need to manually get the `Json` instance from the context, but that is not always obious or easy. In stead, I propose to add a method to the `DeploymentTemplateBuilder`, something like `getDeploymentTemplateJson` that already returnes the json string. To do that, however, we need the Json instance injected in that class. Here it is how this can be achieved:
We'll use Guice Assisted Inject, to be able to have a constructor with both, injected and user provided parameters. It just requires us to define a factory interface and annotate the parameters. Something like:
```java
public class DeploymentTemplateBuilder {
interface Factory {
DeploymentTemplateBuilder create(String group, String name, Template template);
}
...
@Inject // Do not make the constructor public anymore. We will get this from the injector
DeploymentTemplateBuilder(Json json, @Assisted String group, @Assisted String name, @Assisted Template template) {
}
...
}
```
Then, we need to configure the Factory in the Guice context by adding the following to a Guice module:
```java
install(new FactoryModuleBuilder().build(DeploymentTemplateBuilder.Factory.class));
```
And finally, we need to expose the factory in some user-friendly way so users can get it easily. We could add the following to the AzureComputeApi class:
```java
@Provides
DeploymentTemplateBuilder.Factory deploymentTemplateFactory();
```
For methods annotated as `@Provides`, jclouds looks for the values in the Guice injector. Unfortunately if I'm not wrong, that is not yet supported in delegate APIs, so we can't put this method in the DeploymentApi.
Once everything is in place, users should be able to get the json for a deployment template like:
```java
DeploymentTemplateBuilder builder = azureApi.deploymentTemplateFactory().create(group, name, template);
String rawTemplate = builder.getDeploymentTemplateJson();
```
Which looks pretty clean and users don't have to deal with serialization and deserialization at all. WDYT?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-223244620
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + PublicIPAddressProperties.Builder properties = PublicIPAddressProperties.builder();
> +
> + if (!dnsLabelPrefix.isEmpty()) {
> + properties.dnsSettings(DnsSettings.builder().domainNameLabel(dnsLabelPrefix).build());
> + variables.put("dnsLabelPrefix", dnsLabelPrefix);
> + }
> +
> + properties.publicIPAllocationMethod("Dynamic");
> + variables.put("publicIPAddressName", publicIPAddressName);
> + variables.put("publicIPAddressReference", "[resourceId('Microsoft.Network/publicIPAddresses',variables('publicIPAddressName'))]");
> +
> + ResourceDefinition publicIpAddress = ResourceDefinition.builder()
> + .name("[variables('publicIPAddressName')]")
> + .type("Microsoft.Network/publicIPAddresses")
> + .location(location)
> + .apiVersion("2015-06-15")
Add a constant shared with the PublicIpAddresses api
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65267844
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test
> + public void testCreateDeployment() throws Exception
> + {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment accepted
> + server.enqueue(jsonResponse("/createdeploymentaccepted.json"));
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
Use the builders and java objects.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481522
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx The build seems to break on jclouds docker api. Let me know if I need to trigger a fresh build.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-221062190
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
Azure ARM deployment utilizes ARM templates to describe resources to create for a given deployment. `DeploymentApi` is mapped to the [DeploymentApi on Azure](https://msdn.microsoft.com/en-us/library/azure/dn790549.aspx). `DeploymentTemplateBuilder` is the utility to construct the actual ARM template. @jtjk please add anything I may have missed.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64497940
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [fca7addf](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/fca7addf). Many thanks @ritazh!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-223443373
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Jim Spring <no...@github.com>.
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +@AutoValue
> +public abstract class DeploymentTemplate {
> +
> + //Empty placeholders as we want to generate the empty JSON object
> + @AutoValue
> + public abstract static class Parameters {
> + public static Parameters create() {
> + return new AutoValue_DeploymentTemplate_Parameters();
> + }
> + }
> +
> + public abstract String $schema();
@nacx It looks like SerializedNames only goes one direction (deserialization). In order to support this, especially with AutoValue, we need serialization. Maybe I've missed it, but is there support for this?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64838394
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> @@ -66,8 +69,18 @@ public void setup() {
> public void deleteNetworkInterfaceCardResourceDoesNotExist() {
>
> final NetworkInterfaceCardApi nicApi = api.getNetworkInterfaceCardApi(resourcegroup);
> - boolean status = nicApi.delete(NETWORKINTERFACECARD_NAME);
> - assertFalse(status);
> + URI uri = nicApi.delete(NETWORKINTERFACECARD_NAME);
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65107751
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + * List all deployments in a resource group
> + */
> + @Named("deployment:list")
> + @GET
> + @SelectJson("value")
> + @Fallback(EmptyListOnNotFoundOr404.class)
> + List<Deployment> list();
> +
> + /**
> + * The Delete Template Deployment operation starts the process of an ARM Template removal.
> + */
> + @Named("deployment:delete")
> + @DELETE
> + @ResponseParser(URIParser.class)
> + @Path("/{deploymentname}")
> + @Fallback(Fallbacks.VoidOnNotFoundOr404.class)
`NullOnNotFoundOr404`
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64479404
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + .apiVersion("2015-06-15")
> + .properties(
> + StorageServiceProperties.builder()
> + .accountType(StorageService.AccountType.Standard_LRS)
> + .build()
> + )
> + .build();
> +
> + resources.add(storageAccount);
> + }
> +
> + private void addVirtualNetworkResource() {
> + String virtualNetworkName = group + "virtualnetwork";
> + String vnAddresSpacePrefix = "10.0.0.0/16";
> + String subnetName = group + "subnet";
> + String subnetAddressPrefix = "10.0.0.0/24";
+1 to have these values as defaults then!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65012326
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ilkka Salento <no...@github.com>.
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +@AutoValue
> +public abstract class DeploymentTemplate {
> +
> + //Empty placeholders as we want to generate the empty JSON object
> + @AutoValue
> + public abstract static class Parameters {
> + public static Parameters create() {
> + return new AutoValue_DeploymentTemplate_Parameters();
> + }
> + }
> +
> + public abstract String $schema();
DeploymentTemplate is only used to produce JSON string that is passed to the deployment API. It is never used to parse JSON. Adding @SerializedNames does not seem to have desired outcome, after serialization the output is still "{"properties":{"template":{"schema":"https.,". I'll investigate more.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64728087
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Closed #273.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#event-680489584
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + )
> + .subnets(
> + Arrays.asList(
> + Subnet.create("[variables('subnetName')]", null, null,
> + SubnetProperties.builder()
> + .addressPrefix(DEFAULT_subnetAddressPrefix).build()
> + ))
> + )
> + .build();
> +
> +
> + ResourceDefinition virtualNetwork = ResourceDefinition.builder()
> + .name("[variables('virtualNetworkName')]")
> + .type("Microsoft.Network/virtualNetworks")
> + .location(location)
> + .apiVersion("2015-06-15")
Add a constant shared with the network api.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65267758
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
> + protected String storageAccountName;
> + protected String storageAccountType;
It's used by `DeploymentTemplateBuilder` [here](https://github.com/ritazh/jclouds-labs/blob/74f8e9bf66e7427298c5c3c9567aa7d8284e0987/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/DeploymentTemplateBuilder.java#L74)
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64487429
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
Shouldn't this use the DeploymentTemplateBuilder?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64480892
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> +
> + public abstract String $schema();
> +
> + public abstract String contentVersion();
> +
> + public abstract Parameters parameters();
> +
> + public abstract Map<String, String> variables();
> +
> + public abstract List<ResourceDefinition> resources();
> +
> + @Nullable
> + public abstract List<?> outputs();
> +
> + //@SerializedNames({"$schema", "contentVersion", "parameters", "variables", "resources" , "outputs"})
Why is this commented?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64476616
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -114,6 +142,17 @@ private void deleteResourceGroup(String name) {
> api.getResourceGroupApi().delete(name);
> }
>
> + protected ApiContext<AzureComputeApi> getContext() {
> + if (context == null) {
> + Properties properties = new Properties();
> + properties.put("oauth.endpoint", "https://login.microsoftonline.com/tenant-id/oauth2/token");
> + context = ContextBuilder.newBuilder("azurecompute-arm")
> + .credentials(getIdentity(), getCredential())
> + .overrides(properties)
> + .build();
> + }
> + return context;
> + }
Doing this would mean callers will be creating *another* context, just to access the injector. Instead, you'd better override the [BaseApiLiveTest#create](https://github.com/jclouds/jclouds/blob/master/core/src/test/java/org/jclouds/apis/BaseApiLiveTest.java#L141-L145) method, to create the context and assign it before returning the api.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/ed40df18648ddd8602ee3167bf491a05cb14df72#r65508635
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> @@ -30,6 +30,7 @@
> DONE,
> IN_PROGRESS,
> FAILED,
> + NO_CONTENT,
Returns 204-No Content if resource does not exist. https://msdn.microsoft.com/en-us/library/azure/mt163632.aspx Used by `PublicIPAddress`, 'SubnetApi`, and `VirtualNetworkApi`.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64973872
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + }
> + }
> +
> + @AutoValue
> + public abstract static class ProviderResourceType {
> + @Nullable
> + public abstract String resourceType();
> +
> + @Nullable
> + public abstract List<String> locations();
> +
> + @Nullable
> + public abstract List<String> apiVersions();
> +
> + @Nullable
> + public abstract Map<String, JsonBall> properties();
Correct. It can be null.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64495938
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + DeploymentBody body = DeploymentBody.create(template, DEPLOYMENT_MODE, DeploymentTemplate.Parameters.create());
> +
> + return body;
> + }
> +
> +
> + private void addStorageResource() {
> + String storageAccountName = name.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +
> + variables.put("storageAccountName", storageAccountName);
> +
> + ResourceDefinition storageAccount = ResourceDefinition.builder()
> + .name("[variables('storageAccountName')]")
> + .type("Microsoft.Storage/storageAccounts")
> + .location(location)
> + .apiVersion("2015-06-15")
Move this to a constant so the value is shared by the APIs too? (consider this elsewhere).
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64480271
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +@AutoValue
> +public abstract class DeploymentTemplate {
> +
> + //Empty placeholders as we want to generate the empty JSON object
> + @AutoValue
> + public abstract static class Parameters {
> + public static Parameters create() {
> + return new AutoValue_DeploymentTemplate_Parameters();
> + }
> + }
> +
> + public abstract String $schema();
Yes, I know, but having special characters as part of variable names is not very Java. I suggested to name the property "in Java" just `schema` and configure the `@SerializedNames` to be `$schema`, so you can use the right json value while keeping a clean naming in the Java model.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64549964
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + List<SKU> skus = api.getOSImageApi(location).listSKUs(publisher, offer);
> +
> + assertEquals(size(skus), 2);
> +
> + assertSent(server, "GET", requestUrl + "/" + publisher + "/artifacttypes/vmimage/offers/" + offer + "/skus" + apiversion);
> + }
> +
> + public void testVersions() throws InterruptedException {
> + server.enqueue(jsonResponse("/versions.json"));
> +
> + List<Version> versions = api.getOSImageApi(location).listVersions(publisher, offer, sku);
> +
> + assertEquals(size(versions), 2);
> +
> + assertSent(server, "GET", requestUrl + "/" + publisher + "/artifacttypes/vmimage/offers/" + offer + "/skus/" + sku + "/versions" + apiversion);
> + }
Add missing fallback tests.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64482340
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -66,8 +69,18 @@ public void setup() {
> public void deleteNetworkInterfaceCardResourceDoesNotExist() {
>
> final NetworkInterfaceCardApi nicApi = api.getNetworkInterfaceCardApi(resourcegroup);
> - boolean status = nicApi.delete(NETWORKINTERFACECARD_NAME);
> - assertFalse(status);
> + URI uri = nicApi.delete(NETWORKINTERFACECARD_NAME);
We expect this test to return null, so just assert that the URI is actually null and forget about the rest..
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64482012
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
> +
> + Deployment deployment = api().create(deploymentName, properties);
Looking more closely at this method, instead of receiving a json string, shouldn't it receive a DeploymentTemplate parameter?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64481060
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.azurecompute.arm.domain.SKU;
> +import org.jclouds.azurecompute.arm.domain.Version;
> +import org.jclouds.azurecompute.arm.internal.AbstractAzureComputeApiLiveTest;
> +import org.testng.annotations.Test;
> +
> +import static org.testng.Assert.assertTrue;
> +
> +import java.util.List;
> +
> +@Test(groups = "live", testName = "OSImageApiLiveTest")
> +public class OSImageApiLiveTest extends AbstractAzureComputeApiLiveTest {
> +
> + @Test
> + public void testPublisher() {
> + List<Publisher> publishers = api().listPublishers();
> + System.out.println(publishers.size());
Remove all system outs.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64482205
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + }
> + }
> +
> + @AutoValue
> + public abstract static class Provider {
> + @Nullable
> + public abstract String id();
> +
> + @Nullable
> + public abstract String namespace();
> +
> + @Nullable
> + public abstract String registrationState();
> +
> + @Nullable
> + public abstract List<ProviderResourceType> resourceTypes();
Correct. It can be null.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64495948
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
> + protected String storageAccountName;
> + protected String storageAccountType;
Oh, I didn't mean "where" are they used, but "for what purpose" the storageAccountName and storageAccountType options are used, just to understand how the provider is supposed to work :)
(This said, this concrete storageAccountType property seems not to be used in the class you referenced?)
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64488824
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + for (ProvisioningState state : ProvisioningState.values()) {
> + if (text.equalsIgnoreCase(state.key)) {
> + return state;
> + }
> + }
> + }
> + return UNRECOGNIZED;
> + }
> + }
> +
> + public enum DeploymentMode {
> + INCREMENTAL("Incremental"),
> + COMPLETE("Complete"),
> + UNRECOGNIZED("");
> +
> + private final String key;
Same here.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64476104
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + DeploymentBody body = DeploymentBody.create(template, DEPLOYMENT_MODE, DeploymentTemplate.Parameters.create());
> +
> + return body;
> + }
> +
> +
> + private void addStorageResource() {
> + String storageAccountName = name.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +
> + variables.put("storageAccountName", storageAccountName);
> +
> + ResourceDefinition storageAccount = ResourceDefinition.builder()
> + .name("[variables('storageAccountName')]")
> + .type("Microsoft.Storage/storageAccounts")
> + .location(location)
> + .apiVersion("2015-06-15")
I mean, if I get this right, this concerete version should match the version configured in the StorageAccountAPI. I'm just saying to create a constant and use it here and in the Storage API, and use one different constant for each API so it can be reused here, to make sure there are no version mismatches.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65012229
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx Thank you very much for your help! DeploymentTemplateBuilder Factor has been added. Ready for another round of review. :) Almost there!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-223343853
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + variables.put("osDiskName", osDiskName);
> +
> + OSDisk osDisk = OSDisk.builder()
> + .name("[variables('osDiskName')]")
> + .vhd(
> + VHD.create("[concat('http://',variables('storageAccountName'),'.blob.core.windows.net/',variables('storageAccountContainerName'),'/',variables('osDiskName'),'.vhd')]")
> + )
> + .caching("ReadWrite")
> + .createOption("FromImage")
> + .build();
> +
> +
> + //Create Data Disk(s) and add to list
> + final String dataDiskName = name + "datadisk";
> + variables.put("dataDiskName", dataDiskName);
> + final String dataDiskSize = "100";
@jtjk @isalento can you guys please take a look at this?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64496851
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + copyTo(options);
> + return options;
> + }
> +
> + @Override
> + public void copyTo(final TemplateOptions to) {
> + super.copyTo(to);
> + if (to instanceof AzureComputeArmTemplateOptions) {
> + final AzureComputeArmTemplateOptions eTo = AzureComputeArmTemplateOptions.class.cast(to);
> + eTo.virtualNetworkName(virtualNetworkName);
> + if (!subnetNames.isEmpty()) {
> + eTo.subnetNames(subnetNames);
> + }
> + eTo.storageAccountName(storageAccountName);
> + eTo.storageAccountType(storageAccountType);
> + eTo.reservedIPName(reservedIPName);
Missing the security groups property (but don't add it if it is going to be removed as per my previous comment).
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64475211
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + HardwareProfile hw = HardwareProfile.create(vmSize);
> +
> + VirtualMachineProperties properties = VirtualMachineProperties.builder()
> + .hardwareProfile(hw)
> + .osProfile(osProfile)
> + .storageProfile(storageProfile)
> + .networkProfile(networkProfile)
> + .diagnosticsProfile(diagnosticsProfile)
> + .build();
> +
> + variables.put("virtualMachineName", name);
> + ResourceDefinition virtualMachine = ResourceDefinition.builder()
> + .name("[variables('virtualMachineName')]")
> + .type("Microsoft.Compute/virtualMachines")
> + .location(location)
> + .apiVersion("2015-06-15")
Use a shared a constant here too
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65268463
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Jim Spring <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
I happen to disagree, because the actual API is being reflected. There may be cases where an ARM template that isn't exposed via a builder -- maybe Azure Container Service, for instance -- may want to be used. The user should have access to the raw deployment API do do that.
I agree builders are a better abstraction and those, to me, should be at a higher level and not necessarily imposed on the actual API implementation itself.
My biggest two concerns are the imposing of "object oriented" methods on top of a defined API that is basically pre-structured JSON (yes, I know it is error prone); and second, requiring builders reduces the generality of the API, if one were to want to use it in a manner that JClouds (and the arm provider) didn't expose directly.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r65014564
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
@nacx Fixed and sqashed the commits.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-223364764
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Rita Zhang <no...@github.com>.
> + @Override
> + public boolean apply(URI uri) {
> + System.out.println(uri.toString());
> + System.out.println(api.getJobApi().jobStatus(uri).name());
> + return ParseJobStatus.JobStatus.NO_CONTENT == api.getJobApi().jobStatus(uri);
> + }
> + }, 60 * maxTestDuration * 1000).apply(uri);
> + assertTrue(jobDone, "delete operation did not complete in the configured timeout");
> + }
> + }
> + }
> +
> + private DeploymentApi api() {
> + return api.getDeploymentApi(resourceName);
> + }
> +}
Done.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65290782
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + Deployment deployment = deploymentApi.get(deploymentName);
> + assertNull(deployment);
> +
> + assertSent(server, "GET", "/subscriptions/" + subscriptionId + "/resourcegroups/" + resourceGroup +
> + "/providers/microsoft.resources/deployments/" + deploymentName + "?api-version=2016-02-01");
> + }
> +
> + @Test
> + public void testListDeployment() throws Exception {
> + final DeploymentApi deploymentApi = api.getDeploymentApi(resourceGroup);
> +
> + // check if deployment succeeded
> + server.enqueue(jsonResponse("/listdeployments.json"));
> + List<Deployment> deployments = deploymentApi.list();
> + assertTrue(deployments.size() > 0);
Verify the recorded request
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65271208
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
> + protected String storageAccountName;
> + protected String storageAccountType;
> + protected String networkSecurityGroupName;
The template options already provide methods to specify the security groups to be added to the VM. This one seems to be redundant.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64474907
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + ipConfigurations.add(ipConfig);
> +
> + NetworkInterfaceCardProperties networkInterfaceCardProperties = NetworkInterfaceCardProperties.builder()
> + .ipConfigurations(ipConfigurations)
> + .build();
> +
> + String networkInterfaceCardName = name + "nic";
> + variables.put("networkInterfaceCardName", networkInterfaceCardName);
> + variables.put("networkInterfaceCardReference", "[resourceId('Microsoft.Network/networkInterfaces',variables('networkInterfaceCardName'))]");
> +
> + ResourceDefinition networkInterfaceCard = ResourceDefinition.builder()
> + .name("[variables('networkInterfaceCardName')]")
> + .type("Microsoft.Network/networkInterfaces")
> + .location(location)
> + .apiVersion("2015-06-15")
Add a constant here too
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/534e8540c90b57d7d1b6cacd6cad8938788f176a#r65267885
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + * import {@code AzureComputeArmTemplateOptions.*} and invoke a static creation method followed by an instance mutator (if
> + * needed):
> + * <p>
> + *
> + * <pre>
> + * import static org.jclouds.compute.options.AzureComputeArmTemplateOptions.Builder.*;
> + * ComputeService client = // get connection
> + * templateBuilder.options(inboundPorts(22, 80, 8080, 443));
> + * Set<? extends NodeMetadata> set = client.createNodesInGroup(tag, 2, templateBuilder.build());
> + * </pre>
> + *
> + */
> +public class AzureComputeArmTemplateOptions extends TemplateOptions implements Cloneable {
> +
> + protected String virtualNetworkName;
> + protected List<String> subnetNames = ImmutableList.of();
The template options already have a way to provide the IDs of the networks where the VMs have to be attached. Can that property be used instead of having these ones?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64474782
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
Oh, I see it now. The issue is that you're building your custom Gson instance instead of using the jclouds one (configured in the [GsonModule](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/config/GsonModule.java) in jclouds-core). That instance you use does not have the jclouds serialization/deserialization strategies.
You need to use the Json instance provided by the injector, like in my example. Can you try changing the tests to use the jclouds Json instance?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273#issuecomment-222897198
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.json.SerializedNames;
> +
> +@AutoValue
> +public abstract class Deployment {
> +
> + public enum ProvisioningState {
> + ACCEPTED("Accepted"),
> + READY("Ready"),
> + CANCELED("Canceled"),
> + FAILED("Failed"),
> + DELETED("Deleted"),
> + SUCCEEDED("Succeeded"),
> + RUNNING("Running"),
> + UNRECOGNIZED("");
> +
> + private final String key;
This looks redundant. You can omit it and just compare by name()?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64476067
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> + " \"template\" : " + template + ", " +
> + " \"mode\" : \"" + mode + "\", " +
> + " \"parameters\" : " + parameters + " " +
> + " } " +
> + "}";
> + return body;
> + }
> +
> + @Test(groups = "live")
> + public void testCreate() {
> + Long now = System.currentTimeMillis();
> + resourceName = "jcres" + now;
> + deploymentName = "jcdep" + now;
> + String template = "{\"$schema\":\"https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#\",\"contentVersion\":\"1.0.0.0\",\"parameters\":{\"newStorageAccountName\":{\"type\":\"string\",\"metadata\":{\"description\":\"Name of the Storage Account\"}},\"storageAccountType\":{\"type\":\"string\",\"defaultValue\":\"Standard_LRS\",\"allowedValues\":[\"Standard_LRS\",\"Standard_GRS\",\"Standard_ZRS\"],\"metadata\":{\"description\":\"Storage Account type\"}},\"location\":{\"type\":\"string\",\"allowedValues\":[\"East US\",\"West US\",\"West Europe\",\"East Asia\",\"Southeast Asia\"],\"metadata\":{\"description\":\"Location of storage account\"}}},\"resources\":[{\"type\":\"Microsoft.Storage/storageAccounts\",\"name\":\"[parameters('newStorageAccountName')]\",\"apiVersion\":\"2015-05-01-preview\",\"location\":\"[parameters('location')]\",\"properties\":{\"accountType\":\"[parameters('storageAccountType')]\"}}]}";
> + String parameters = "{\"newStorageAccountName\":{\"value\":\"" + resourceName + "\"},\"storageAccountType\":{\"value\":\"Standard_LRS\"},\"location\":{\"value\":\"West US\"}}";
> + String properties = getPutBody(template, "Incremental", parameters);
Yes, I understand that. What I mean is that a deployment template is actually a json that goes directly in the request body. Despite the fact that it is an ARM template (conceptually), there is no practical differente with other API requests. You are just sending a request with a json body.
But the user experience, in this case is worse: Users have to build on their own that json. They can use the builders, but they have to manually serialize the template into a json, just because our `DeploymentApi` methods expect the template to be a json String.
What I wanted to say is that the DeploymentApi methods should be changed to receive a "DeploymentTemplate" (or whatever name it is appropriate) object, instead of a raw json, as it does now with the "properties" string. Let jclouds serialize the ARM template, but keep users just using high level objects. They shouldn't have to worry about generating raw json.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64550524
Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi
OSImageApi VMSizeApi (#273)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import java.util.List;
> +import java.util.Map;
> +
> +@AutoValue
> +public abstract class DeploymentTemplate {
> +
> + //Empty placeholders as we want to generate the empty JSON object
> + @AutoValue
> + public abstract static class Parameters {
> + public static Parameters create() {
> + return new AutoValue_DeploymentTemplate_Parameters();
> + }
> + }
> +
> + public abstract String $schema();
Can we remove the `$` character fro the variable?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/273/files/74f8e9bf66e7427298c5c3c9567aa7d8284e0987#r64476640