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/06/03 01:31:28 UTC

[jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm Azurecompute-arm compute service (#279)

@nacx @andreaturli 
This PR is for Azure ARM Compute Service, 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/279

-- Commit Summary --

  * JCLOUDS-664 Azurecompute-arm Azurecompute-arm compute service

-- File Changes --

    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeApi.java (11)
    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureManagementApiMetadata.java (2)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeServiceAdapter.java (429)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzureComputeServiceContextModule.java (127)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/functions/DeploymentToNodeMetadata.java (162)
    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/functions/LocationToLocation.java (7)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/functions/VMHardwareToHardware.java (79)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/functions/VMImageToImage.java (126)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/ResourceProviderMetaData.java (68)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/VMHardware.java (66)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/VMImage.java (50)
    A azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/features/ResourceProviderApi.java (57)
    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/ResourceProviderAPIMockTest.java (53)
    A azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/ResourceProviderApiLiveTest.java (56)
    A azurecompute-arm/src/test/resources/getresourceprovidermetadata.json (366)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/279.patch
https://github.com/jclouds/jclouds-labs/pull/279.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/279

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

Where are you destroying the deployment? Does the destroyDeployment takes care of deleting all the cloud resources created for this deployment?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65839954

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +   }
> +
> +   @Override
> +   public Image apply(final VMImage image) {
> +      FluentIterable.from(locations.get()).firstMatch(LocationPredicates.idEquals(image.location)).orNull();
> +
> +      final ImageBuilder builder = new ImageBuilder()
> +              .name(image.offer)
> +              .description(image.sku)
> +              .status(Image.Status.AVAILABLE)
> +              .version(image.sku)
> +              .id(encodeFieldsToUniqueId(image))
> +              .providerId(image.publisher)
> +              .location(image.globallyAvailable ? null : FluentIterable.from(locations.get())
> +                      .firstMatch(LocationPredicates.idEquals(image.location))
> +                      .orNull());

Same comment about failing instead of returning 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65967958

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
@andreaturli @nacx Occasionally I'm getting the following error now. Any idea or recommendations?

org.jclouds.rest.AuthorizationException: (jclouds:pw[0cef1fb10f60529028a71f58e54ed07b]@[hostip]:22) (jclouds:pw[0cef1fb10f60529028a71f58e54ed07b]@[hostip:22) error acquiring {hostAndPort=[hostip:22, loginUser=jclouds, ssh=null, connectTimeout=60000, sessionTimeout=60000} (not retryable): Exhausted available authentication methods


---
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/279#issuecomment-224640560

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
_bump_ @nacx we also have few more features that depend on this getting merged. (For example, capturing image, creating VM with custom images, and getting the jclouds-example app working against this provider) If there are missing tests, let us know and we will get those created in a followup PR. Thank you!

---
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/279#issuecomment-223866564

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +            }
> +         }, 60 * 10 * 1000 /* 5 minute timeout */).apply(uri);
> +
> +         if (jobDone) {
> +            // Delete storage account
> +            api.getStorageAccountApi(azureGroup).delete(storageAccountName);
> +
> +            // Delete NIC
> +            uri = api.getNetworkInterfaceCardApi(azureGroup).delete(id + "nic");
> +            if (uri != null){
> +               jobDone = Predicates2.retry(new Predicate<URI>() {
> +                  @Override public boolean apply(URI uri) {
> +                     return ParseJobStatus.JobStatus.DONE == api.getJobApi().jobStatus(uri)
> +                             || ParseJobStatus.JobStatus.NO_CONTENT == api.getJobApi().jobStatus(uri);
> +                  }
> +               }, 60 * 10 * 1000 /* 5 minute timeout */).apply(uri);

Same here, move to a reusable predicate and get it injected.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65965559

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +@AutoValue
> +public abstract class ResourceProviderMetaData {
> +
> +   public abstract String resourceType();
> +
> +   public abstract List<String> locations();
> +
> +   public abstract List<String> apiVersions();
> +
> +   @SerializedNames({"resourceType", "locations", "apiVersions"})
> +   public static ResourceProviderMetaData create(final String resourceType, final List<String> locations, final List<String> apiVersions) {
> +      ResourceProviderMetaData.Builder builder = ResourceProviderMetaData.builder()
> +              .resourceType(resourceType)
> +              .locations(locations == null ? null : ImmutableList.copyOf(locations))
> +              .apiVersions(apiVersions == null ? null : ImmutableList.copyOf(apiVersions));

If fields can be really nullable, then annotate the fields accordingly.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65968111

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +      return (imageReference.globallyAvailable ? "global" : imageReference.location) + "/" + imageReference.publisher + "/" + imageReference.offer + "/" + imageReference.sku;
> +   }
> +
> +   public static String[] decodeFieldsFromUniqueId(final String id) {
> +      final String[] parts = checkNotNull(id, "id").split("/");
> +      return parts;
> +   }
> +
> +   @Inject
> +   VMImageToImage(@Memoized final Supplier<Set<? extends Location>> locations) {
> +      this.locations = locations;
> +   }
> +
> +   @Override
> +   public Image apply(final VMImage image) {
> +      FluentIterable.from(locations.get()).firstMatch(LocationPredicates.idEquals(image.location)).orNull();

Remove 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65967902

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
Thanks @ritazh, just wondering, do you think this
`-Dtest.jclouds.oauth.resource="https://management.azure.com/` parameter is likely to change? or it will be always the same? in that case, maybe we should add it to the [ProviderMetadata](https://github.com/jclouds/jclouds-labs/blob/2b4d283ce54f67114a64a6dad11df01a8d5ec324/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/AzureComputeProviderMetadata.java#L55-L55)



---
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/279#issuecomment-224310782

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +
> +   @Inject
> +   VMHardwareToHardware(@Memoized final Supplier<Set<? extends Location>> locations) {
> +      this.locations = locations;
> +   }
> +
> +   @Override
> +   public Hardware apply(VMHardware from) {
> +      final HardwareBuilder builder = new HardwareBuilder()
> +              .name(from.name)
> +              .id(from.name)
> +              .processors(ImmutableList.of(new Processor(from.numberOfCores, 2)))
> +              .ram(from.memoryInMB)
> +              .location(from.globallyAvailable ? null : FluentIterable.from(locations.get())
> +                      .firstMatch(LocationPredicates.idEquals(from.location))
> +                      .orNull());

@nacx By fail, do you mean an exception? If so, what type of exception 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66018162

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, DEFAULT_LOGIN_PASSWORD);
> +      }
> +      builder.credentials(LoginCredentials.fromCredentials(credentials));
> +
> +      final Set<String> publicIpAddresses = Sets.newLinkedHashSet();
> +      if (from.ipAddressList != null) {
> +         for (int c = 0; c < from.ipAddressList.size(); c++) {
> +            PublicIPAddress ip = from.ipAddressList.get(c);
> +            if (ip != null && ip.properties() != null && ip.properties().ipAddress() != null)
> +               publicIpAddresses.add(ip.properties().ipAddress());
> +         }
> +         if (publicIpAddresses.size() > 0)
> +            builder.publicAddresses(publicIpAddresses);
> +      }
> +
> +      return builder.build();

Also set the image, location and hardware

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65967471

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [03f72d5e](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/03f72d5e). Fantastic work done here @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/279#issuecomment-226880122

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +                   put(Deployment.ProvisioningState.RUNNING, NodeMetadata.Status.PENDING).
> +                   put(Deployment.ProvisioningState.CANCELED, NodeMetadata.Status.TERMINATED).
> +                   put(Deployment.ProvisioningState.FAILED, NodeMetadata.Status.ERROR).
> +                   put(Deployment.ProvisioningState.DELETED, NodeMetadata.Status.TERMINATED).
> +                   put(Deployment.ProvisioningState.SUCCEEDED, NodeMetadata.Status.RUNNING).
> +                   put(Deployment.ProvisioningState.UNRECOGNIZED, NodeMetadata.Status.UNRECOGNIZED).
> +                   build();
> +
> +   public static Deployment.ProvisioningState provisioningStateFromString(final String text) {
> +      if (text != null) {
> +         for (Deployment.ProvisioningState status : Deployment.ProvisioningState.values()) {
> +            if (text.equalsIgnoreCase(status.name())) {
> +               return status;
> +            }
> +         }
> +      }

Use the existing GetEnumValue helper?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65966528

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

Sure thing! Let me give that a try! 👍 

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65922235

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

Thanks @ritazh I think this is interesting. In Openstack we have implemented a roll-back operation (see [link](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java#L152) for more details, which can be an interesting pattern for ARM as well. 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65913873

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_POLL_INITIAL_PERIOD;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_POLL_MAX_PERIOD;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TCP_RULE_FORMAT;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TCP_RULE_REGEXP;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.DEFAULT_IMAGE_LOGIN;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TIMEOUT_RESOURCE_DELETED;
> +
> +public class AzureComputeServiceContextModule
> +        extends ComputeServiceAdapterContextModule<VMDeployment, VMHardware, VMImage, Location> {
> +
> +   @Override
> +   protected void configure() {
> +      super.configure();
> +      bind(new TypeLiteral<ComputeServiceAdapter<VMDeployment, VMHardware, VMImage, Location>>() {
> +      }).to(AzureComputeServiceAdapter.class);
> +      bind(new TypeLiteral<Function<VMImage, org.jclouds.compute.domain.Image>>() {

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/279/files/964b44968c43b0671f945b6c0538ec32f8794052#r66665300

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +   }
> +
> +   private List<PublicIPAddress> getIPAddresses(Deployment deployment) {
> +      List<PublicIPAddress> list = new ArrayList<PublicIPAddress>();
> +      String resourceGroup = getResourceGroupFromId(deployment.id());
> +
> +      if (deployment.properties() != null && deployment.properties().dependencies() != null) {
> +         List<Deployment.Dependency> dependencies = deployment.properties().dependencies();
> +         for (int d = 0; d < dependencies.size(); d++) {
> +            if (dependencies.get(d).resourceType().equals("Microsoft.Network/networkInterfaces")) {
> +               List<Deployment.Dependency> dependsOn = dependencies.get(d).dependsOn();
> +               for (int e = 0; e < dependsOn.size(); e++) {
> +                  if (dependsOn.get(e).resourceType().equals("Microsoft.Network/publicIPAddresses")) {
> +                     String resourceName = dependsOn.get(e).resourceName();
> +                     PublicIPAddress ip = api.getPublicIPAddressApi(resourceGroup).get(resourceName);
> +                     list.add(ip);

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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66017869

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +      view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +   }
> +
> +   @Test(dependsOnMethods = "testRestart")
> +   public void testLinuxNode() throws RunNodesException {
> +      final String groupName = String.format("ubu%s", System.getProperty("user.name").substring(0, 3));
> +      final TemplateBuilder templateBuilder = view.getComputeService().templateBuilder();
> +      templateBuilder.osFamily(OsFamily.UBUNTU);
> +      templateBuilder.osVersionMatches("14.04");
> +      templateBuilder.hardwareId("Standard_A0");
> +      templateBuilder.locationId("westus");
> +      final Template template = templateBuilder.build();
> +
> +      final TemplateOptions options = template.getOptions();
> +      options.inboundPorts(5985);

Is this relevant for this test?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65969372

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> @@ -207,11 +207,11 @@ private void getImagesFromPublisher(String publisherName, List<VMImage> osImages
>  
>     private List<VMImage> listImagesByLocation(String location) {
>        final List<VMImage> osImages = Lists.newArrayList();
> -
> -      getImagesFromPublisher("Microsoft.WindowsAzure.Compute", osImages, location);
> -      getImagesFromPublisher("MicrosoftWindowsServer", osImages, location);
> -      getImagesFromPublisher("Canonical", osImages, location);
> -
> +      String[] publishers = this.azureComputeConstants.azureImagePublishers().split(",");

consider something like
``
Iterable<String> result = Splitter.on(',')
	       .trimResults()
	       .omitEmptyStrings()
	       .split(str);
``

---
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/279/files/0a3d33807b5b9ea5209577bca2897f5342f682c8..aa9878bbe3563ab696a61f54b0d2812ebc6c5d97#r66136919

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +   public static String NAME_PREFIX = "azu%s";
> +
> +   @Override
> +   protected Module getSshModule() {
> +      return new SshjSshClientModule();
> +   }
> +
> +   @Override protected Properties setupProperties() {
> +      Properties properties = super.setupProperties();
> +
> +      properties.put(ComputeServiceProperties.POLL_INITIAL_PERIOD, 1000);
> +      properties.put(ComputeServiceProperties.POLL_MAX_PERIOD, 10000);
> +      properties.setProperty(AzureComputeProperties.OPERATION_TIMEOUT, "46000000");
> +      properties.setProperty(AzureComputeProperties.OPERATION_POLL_INITIAL_PERIOD, "5");
> +      properties.setProperty(AzureComputeProperties.OPERATION_POLL_MAX_PERIOD, "15");

Are these sane defaults? In that case, is it worth moving them to the api metadata?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65968482

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

Thanks @ritazh! 
I think you can extract the `destroyNode` logic into a function like [CleanupServer](https://github.com/jclouds/jclouds/blob/a4a255fa4ae6c7a1a46aa2a8420b19cd2b7df463/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CleanupServer.java)
which is actually a `CleanupAccount` :)

This can be used in the `destroyNode` and in `AzureComputeService` similarly to [this](https://github.com/jclouds/jclouds/blob/a4a255fa4ae6c7a1a46aa2a8420b19cd2b7df463/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeService.java#L99-L99)

makes sense?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66079610

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +      server.enqueue(jsonResponse("/getresourceprovidermetadata.json"));
> +
> +      final ResourceProviderApi resourceProviderApi = api.getResourceProviderApi();
> +
> +      List<ResourceProviderMetaData> metaDatas = resourceProviderApi.get(resource);
> +
> +      String path = String.format("/subscriptions/SUBSCRIPTIONID/providers/%s?api-version=%s", resource, apiVersion);
> +
> +      assertSent(server, "GET", path);
> +      assertTrue(metaDatas.size() > 0);
> +      ResourceProviderMetaData md = metaDatas.get(0);
> +
> +      assertEquals(md.resourceType(), "availabilitySets");
> +      assertEquals(md.locations().get(0), "East US");
> +      assertEquals(md.apiVersions().get(0), "2016-03-30");
> +   }

Added.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66018427

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
hi @ritazh, I'm trying to follow the [README](https://github.com/jclouds/jclouds-labs/blob/8e7d4c057640151ebefbfbd3ef24de8be7a9171f/azurecompute-arm/README.md#run-live-tests) to run the liveTests but I get this error:
```
java.lang.NullPointerException: test.jclouds.oauth.resource

	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:229)
	at org.jclouds.azurecompute.arm.compute.AzureComputeServiceContextLiveTest.setupProperties(AzureComputeServiceContextLiveTest.java:74)
	at org.jclouds.apis.BaseViewLiveTest.initializeContext(BaseViewLiveTest.java:37)
	at org.jclouds.apis.BaseContextLiveTest.setupContext(BaseContextLiveTest.java:74)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
	at org.testng.internal.Invoker.invokeConfigurationMethod(Invoker.java:552)
	at org.testng.internal.Invoker.invokeConfigurations(Invoker.java:215)
	at org.testng.internal.Invoker.invokeConfigurations(Invoker.java:140)
	at org.testng.internal.TestMethodWorker.invokeBeforeClassMethods(TestMethodWorker.java:170)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:104)
	at org.testng.TestRunner.privateRun(TestRunner.java:767)
	at org.testng.TestRunner.run(TestRunner.java:617)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:348)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:343)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:305)
	at org.testng.SuiteRunner.run(SuiteRunner.java:254)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1224)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1149)
	at org.testng.TestNG.run(TestNG.java:1057)
	at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:74)
	at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:121)
```

Can you please update the README as well? 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/279#issuecomment-224266056

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +      } finally {
> +         view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +      }
> +   }
> +
> +   @Test(dependsOnMethods = "testLinuxNode")
> +   public void testWindowsNode() throws RunNodesException {
> +      final String groupName = String.format("win%s", System.getProperty("user.name").substring(0, 3));
> +      final TemplateBuilder templateBuilder = view.getComputeService().templateBuilder();
> +      templateBuilder.imageId("westus/MicrosoftWindowsServer/WindowsServer/2016-Technical-Preview-with-Containers");
> +      templateBuilder.hardwareId("Standard_A0");
> +      templateBuilder.locationId("westus");
> +      final Template template = templateBuilder.build();
> +
> +      final TemplateOptions options = template.getOptions().as(TemplateOptions.class);
> +      options.inboundPorts(5985);

Same here, is this relevant for this test?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65969409

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
Absolutely @nacx, +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/279#issuecomment-226687047

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> +
> +   @Inject
> +   VMHardwareToHardware(@Memoized final Supplier<Set<? extends Location>> locations) {
> +      this.locations = locations;
> +   }
> +
> +   @Override
> +   public Hardware apply(VMHardware from) {
> +      final HardwareBuilder builder = new HardwareBuilder()
> +              .name(from.name)
> +              .id(from.name)
> +              .processors(ImmutableList.of(new Processor(from.numberOfCores, 2)))
> +              .ram(from.memoryInMB)
> +              .location(from.globallyAvailable ? null : FluentIterable.from(locations.get())
> +                      .firstMatch(LocationPredicates.idEquals(from.location))
> +                      .orNull());

I guess @nacx meant that if you have orNull in the second option then it will be equals to the first option.
I think it is better to analyse the location outside the builder and fail-fast, maybe something like [this](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/compute/functions/DropletToNodeMetadata.java#L157) ?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66057522

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
Thanks @andreaturli You are right. It's already in ProviderMetadata 
`properties.put(RESOURCE, "https://management.azure.com/");`
Updated `AbstractAzureComputeApiLiveTest` to remove the check.

---
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/279#issuecomment-224313347

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +      super.configure();
> +
> +      bind(new TypeLiteral<ComputeServiceAdapter<VMDeployment, VMHardware, VMImage, Location>>() {
> +      }).to(AzureComputeServiceAdapter.class);
> +      bind(new TypeLiteral<Function<VMImage, org.jclouds.compute.domain.Image>>() {
> +      }).to(VMImageToImage.class);
> +      bind(new TypeLiteral<Function<VMHardware, Hardware>>() {
> +      }).to(VMHardwareToHardware.class);
> +      bind(new TypeLiteral<Function<VMDeployment, NodeMetadata>>() {
> +      }).to(DeploymentToNodeMetadata.class);
> +
> +      //bind(PrioritizeCredentialsFromTemplate.class).to(UseNodeCredentialsButOverrideFromTemplate.class);
> +      bind(new TypeLiteral<Function<Location, org.jclouds.domain.Location>>() {
> +      }).to(LocationToLocation.class);
> +
> +      //bind(CreateNodesInGroupThenAddToSet.class).to(GetOrCreateStorageServiceAndVirtualNetworkThenCreateNodes.class);

Remove commented lines

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65965837

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +      ResourceGroup resourceGroup = resourceGroupApi.get(azureGroup);
> +      final String resourceGroupName;
> +
> +      if (resourceGroup == null){
> +         final Map<String, String> tags = ImmutableMap.of("description", "jClouds managed VMs");
> +         resourceGroupName = resourceGroupApi.create(azureGroup, location, tags).name();
> +      } else {
> +         resourceGroupName = resourceGroup.name();
> +      }
> +
> +      if (!retry(new Predicate<String>() {
> +         @Override
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);

Get the deployment api outside the loop to save the class lookup in every iteration.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65964314

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, DEFAULT_LOGIN_PASSWORD);
> +      }
> +      builder.credentials(LoginCredentials.fromCredentials(credentials));
> +
> +      final Set<String> publicIpAddresses = Sets.newLinkedHashSet();
> +      if (from.ipAddressList != null) {
> +         for (int c = 0; c < from.ipAddressList.size(); c++) {
> +            PublicIPAddress ip = from.ipAddressList.get(c);
> +            if (ip != null && ip.properties() != null && ip.properties().ipAddress() != null)
> +               publicIpAddresses.add(ip.properties().ipAddress());
> +         }
> +         if (publicIpAddresses.size() > 0)
> +            builder.publicAddresses(publicIpAddresses);
> +      }
> +
> +      return builder.build();

@nacx I have added these. Please take a look at dec3b7f 

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66492135

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +      this.vmImageToImage = vmImageToImage;
> +      this.vmHardwareToHardware = vmHardwareToHardware;
> +      this.credentialStore = credentialStore;
> +      this.api = api;
> +   }
> +
> +   @Override
> +   public NodeMetadata apply(final VMDeployment from) {
> +      final NodeMetadataBuilder builder = new NodeMetadataBuilder();
> +      final Deployment deployment = from.deployment;
> +      builder.id(deployment.name());
> +      builder.providerId(deployment.name());
> +      builder.name(deployment.name());
> +      String group = deployment.name();
> +      int index = group.lastIndexOf("-");
> +      builder.group(group.substring(0, index));

If this is parsing a jclouds generated name, then the group naming convention already has methods to get the group part. Use that, since the separator used by the convention is also configurable by 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65966810

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +   @Override
> +   protected Module getSshModule() {
> +      return new SshjSshClientModule();
> +   }
> +
> +   @Override protected Properties setupProperties() {
> +      Properties properties = super.setupProperties();
> +
> +      properties.put(ComputeServiceProperties.POLL_INITIAL_PERIOD, 1000);
> +      properties.put(ComputeServiceProperties.POLL_MAX_PERIOD, 10000);
> +      properties.setProperty(AzureComputeProperties.OPERATION_TIMEOUT, "46000000");
> +      properties.setProperty(AzureComputeProperties.OPERATION_POLL_INITIAL_PERIOD, "5");
> +      properties.setProperty(AzureComputeProperties.OPERATION_POLL_MAX_PERIOD, "15");
> +      properties.setProperty(AzureComputeProperties.TCP_RULE_FORMAT, "tcp_%s-%s");
> +      properties.setProperty(AzureComputeProperties.TCP_RULE_REGEXP, "tcp_\\d{1,5}-\\d{1,5}");

Are these tcp properties actually used?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65968385

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

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;
> +
> +/**
> + * A VM Size that is available in a region for a given subscription.
> + *
> + * @see <a href="https://msdn.microsoft.com/en-us/library/azure/mt269440.aspx" >api</a>
> + */
> +
> +public class VMHardware {

Use google auto for internal classes 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65968236

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +            VMImage vmImage = new VMImage();
> +            vmImage.publisher = publisherName;
> +            vmImage.offer = offer.name();
> +            vmImage.sku = sku.name();
> +            vmImage.location = location;
> +            osImagesRef.add(vmImage);
> +         }
> +      }
> +   }
> +
> +   private List<VMImage> listImagesByLocation(String location) {
> +      final List<VMImage> osImages = Lists.newArrayList();
> +
> +      getImagesFromPublisher("Microsoft.WindowsAzure.Compute", osImages, location);
> +      getImagesFromPublisher("MicrosoftWindowsServer", osImages, location);
> +      getImagesFromPublisher("Canonical", osImages, location);

Avoid hardcoding this, and configure a comma-separated property instead, so users can customize the publishers? These three values should be preconfigured as a default in the azure api metadata.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65964913

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +               try {
> +                  Thread.sleep(30 * 1000);
> +               } catch (InterruptedException e) {
> +               }
> +               continue;
> +            }
> +            else
> +            {
> +               allRestarted = true;
> +            }
> +         }
> +      }
> +      assertTrue(allRestarted);
> +
> +      view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +   }

@nacx Please take a look at [this](https://github.com/jclouds/jclouds-labs/pull/279/commits/dd1245789aeeb364c0fcdcf50c8444448b359e1d) commit.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66198142

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
@nacx @andreaturli Lots of changes in for this PR. Ready for another round of review. 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/279#issuecomment-225096777

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +   @Override
> +   public void destroyNode(final String id) {
> +      logger.debug("Destroying %s ...", id);
> +      String storageAccountName = id.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +      int index = id.lastIndexOf("-");
> +      String group = id.substring(0, index);
> +
> +      // Delete VM
> +      URI uri = api.getVirtualMachineApi(azureGroup).delete(id);
> +      if (uri != null){
> +         boolean jobDone = Predicates2.retry(new Predicate<URI>() {
> +            @Override public boolean apply(URI uri) {
> +               return ParseJobStatus.JobStatus.DONE == api.getJobApi().jobStatus(uri);
> +            }
> +         }, 60 * 10 * 1000 /* 5 minute timeout */).apply(uri);

Refactor this into a reusable predicate (see https://github.com/jclouds/jclouds-labs/pull/280#discussion_r65962712). This will also allow users to configure these timeouts and polling.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65965430

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66017830

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +            VMImage vmImage = new VMImage();
> +            vmImage.publisher = publisherName;
> +            vmImage.offer = offer.name();
> +            vmImage.sku = sku.name();
> +            vmImage.location = location;
> +            osImagesRef.add(vmImage);
> +         }
> +      }
> +   }
> +
> +   private List<VMImage> listImagesByLocation(String location) {
> +      final List<VMImage> osImages = Lists.newArrayList();
> +
> +      getImagesFromPublisher("Microsoft.WindowsAzure.Compute", osImages, location);
> +      getImagesFromPublisher("MicrosoftWindowsServer", osImages, location);
> +      getImagesFromPublisher("Canonical", osImages, location);

Right!

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66020456

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

@andreaturli Just added this "feature". Please take a look at 6f55f45

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66364701

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_POLL_INITIAL_PERIOD;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_POLL_MAX_PERIOD;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TCP_RULE_FORMAT;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TCP_RULE_REGEXP;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.DEFAULT_IMAGE_LOGIN;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TIMEOUT_RESOURCE_DELETED;
> +
> +public class AzureComputeServiceContextModule
> +        extends ComputeServiceAdapterContextModule<VMDeployment, VMHardware, VMImage, Location> {
> +
> +   @Override
> +   protected void configure() {
> +      super.configure();
> +      bind(new TypeLiteral<ComputeServiceAdapter<VMDeployment, VMHardware, VMImage, Location>>() {
> +      }).to(AzureComputeServiceAdapter.class);
> +      bind(new TypeLiteral<Function<VMImage, org.jclouds.compute.domain.Image>>() {

As you added `AzureComputeService` you need to bind it in the ContextModule with something like
```
      bind(ComputeService.class).to(AzureComputeService.class);
```


---
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/279/files/964b44968c43b0671f945b6c0538ec32f8794052#r66583837

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_POLL_INITIAL_PERIOD;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.OPERATION_POLL_MAX_PERIOD;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TCP_RULE_FORMAT;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TCP_RULE_REGEXP;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.DEFAULT_IMAGE_LOGIN;
> +import static org.jclouds.azurecompute.arm.config.AzureComputeProperties.TIMEOUT_RESOURCE_DELETED;
> +
> +public class AzureComputeServiceContextModule
> +        extends ComputeServiceAdapterContextModule<VMDeployment, VMHardware, VMImage, Location> {
> +
> +   @Override
> +   protected void configure() {
> +      super.configure();
> +      bind(new TypeLiteral<ComputeServiceAdapter<VMDeployment, VMHardware, VMImage, Location>>() {
> +      }).to(AzureComputeServiceAdapter.class);
> +      bind(new TypeLiteral<Function<VMImage, org.jclouds.compute.domain.Image>>() {

@andreaturli Added it in commit c944c77

---
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/279/files/964b44968c43b0671f945b6c0538ec32f8794052#r66637964

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
Thanks @ritazh! The structure is pretty good, and it looks like the implementation is already in the right direction. There is a great job in here!

>If there are missing tests, let us know and we will get those created in a followup PR. Thank you!

I already commented the two test base classes that have to be implemented and passing in order to consider the provider ready. I think the current implementation is pretty close to having them! Once you override those, you'll see some tests fail due to credentials, others due to options, etc. Don't get stuck and ping me if you need help! Once those tests pass, we can say the provider is ready to go, even if the extensions are not there 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/279#issuecomment-224090738

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +   @Test
> +   public void testGetComputeProviderMetadata() {
> +
> +      List<ResourceProviderMetaData> computeMetaData = api().get(PROVIDER);
> +
> +      assertNotNull(computeMetaData);
> +
> +      boolean foundVMResource = false;
> +
> +      for (ResourceProviderMetaData m : computeMetaData) {
> +         if (m.resourceType().equals(VM_RESOURCE_TYPE)) {
> +            foundVMResource = true;
> +            break;
> +         }
> +      }
> +      assertTrue(foundVMResource);

Use iterables.any?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65970075

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.logging.Logger;
> +
> +import com.google.common.base.Predicate;
> +import com.google.common.collect.Iterables;
> +import com.google.common.collect.Lists;
> +import com.google.common.collect.Sets;
> +import org.jclouds.util.Predicates2;
> +
> +/**
> + * Defines the connection between the {@link AzureComputeApi} implementation and the jclouds
> + * {@link org.jclouds.compute.ComputeService}.
> + */
> +@Singleton
> +public class AzureComputeServiceAdapter implements ComputeServiceAdapter<VMDeployment, VMHardware, VMImage, Location> {
> +
> +   private static int runningNumber = 1;

We need to get rid of this shared mutable state

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65963225

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
The structure looks good. There are several things such as node credentials and other stuff that has to be refactored, but I think it is in a good state to merge.
Subsequent pull requests should add the mentioned live tests, and those will force the implementation to change things to be done the right way :)

@ritazh Mind squashing the commits so we can merge the PR?
@andreaturli Are you OK with merging this to set the foundation, and add the tests and functional fixes to  get them passing in a subsequent PR?

---
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/279#issuecomment-226624138

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
@ritazh generally this means that jclouds couldn't ssh to the VM for the following reasons:
```
- timeout: slow provisioning
- wrong password
- port 22 closed
```
could you identify a test case that is likely to show the problem more often?

---
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/279#issuecomment-224641891

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +
> +   @Override
> +   public void destroyNode(final String id) {
> +      logger.debug("Destroying %s ...", id);
> +      String storageAccountName = id.replaceAll("[^A-Za-z0-9 ]", "") + "storage";
> +      int index = id.lastIndexOf("-");
> +      String group = id.substring(0, index);
> +
> +      // Delete VM
> +      URI uri = api.getVirtualMachineApi(azureGroup).delete(id);
> +      if (uri != null){
> +         boolean jobDone = Predicates2.retry(new Predicate<URI>() {
> +            @Override public boolean apply(URI uri) {
> +               return ParseJobStatus.JobStatus.DONE == api.getJobApi().jobStatus(uri);
> +            }
> +         }, 60 * 10 * 1000 /* 5 minute timeout */).apply(uri);

@nacx Can you please take a look at [this](https://github.com/jclouds/jclouds-labs/pull/279/commits/4256a43cd5cd8737e3769a5cfbbf28be7fac02fa) commit

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66205663

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
@andreaturli Updated the README

---
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/279#issuecomment-224307557

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +      } finally {
> +         view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +      }
> +   }
> +
> +   @Test(dependsOnMethods = "testLinuxNode")
> +   public void testWindowsNode() throws RunNodesException {
> +      final String groupName = String.format("win%s", System.getProperty("user.name").substring(0, 3));
> +      final TemplateBuilder templateBuilder = view.getComputeService().templateBuilder();
> +      templateBuilder.imageId("westus/MicrosoftWindowsServer/WindowsServer/2016-Technical-Preview-with-Containers");
> +      templateBuilder.hardwareId("Standard_A0");
> +      templateBuilder.locationId("westus");
> +      final Template template = templateBuilder.build();
> +
> +      final TemplateOptions options = template.getOptions().as(TemplateOptions.class);
> +      options.inboundPorts(5985);

Removed.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66018318

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +      server.enqueue(jsonResponse("/getresourceprovidermetadata.json"));
> +
> +      final ResourceProviderApi resourceProviderApi = api.getResourceProviderApi();
> +
> +      List<ResourceProviderMetaData> metaDatas = resourceProviderApi.get(resource);
> +
> +      String path = String.format("/subscriptions/SUBSCRIPTIONID/providers/%s?api-version=%s", resource, apiVersion);
> +
> +      assertSent(server, "GET", path);
> +      assertTrue(metaDatas.size() > 0);
> +      ResourceProviderMetaData md = metaDatas.get(0);
> +
> +      assertEquals(md.resourceType(), "availabilitySets");
> +      assertEquals(md.locations().get(0), "East US");
> +      assertEquals(md.apiVersions().get(0), "2016-03-30");
> +   }

Add a mock test that exercises the 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65970019

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +               try {
> +                  Thread.sleep(30 * 1000);
> +               } catch (InterruptedException e) {
> +               }
> +               continue;
> +            }
> +            else
> +            {
> +               allRestarted = true;
> +            }
> +         }
> +      }
> +      assertTrue(allRestarted);
> +
> +      view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +   }

All the above tests are already covered by the `BaseComputeServiceLiveTest`. You should subclass that one and try to make all tests pass without overriding its methods (as that is our compute service implementation contract). Take the [DigitalOcean](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/test/java/org/jclouds/digitalocean2/compute/DigitalOcean2ComputeServiceLiveTest.java) one as an example.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65969303

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +               } else if (statuses.get(c).displayStatus().equals("VM stopped")) {
> +                  status = NodeMetadata.Status.SUSPENDED;
> +               }
> +               break;
> +            }
> +         }
> +      }
> +
> +      builder.status(status);
> +
> +      Credentials credentials = credentialStore.get("node#" + from.deployment.name());
> +      if (credentials != null && credentials.identity.equals(JCLOUDS_DEFAULT_USERNAME)) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, credentials.credential);
> +      } else if (credentials == null) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, DEFAULT_LOGIN_PASSWORD);
> +      }

Hmmm I don't quite get this. Why are you changing the username 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65967219

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +            VMImage vmImage = new VMImage();
> +            vmImage.publisher = publisherName;
> +            vmImage.offer = offer.name();
> +            vmImage.sku = sku.name();
> +            vmImage.location = location;
> +            osImagesRef.add(vmImage);
> +         }
> +      }
> +   }
> +
> +   private List<VMImage> listImagesByLocation(String location) {
> +      final List<VMImage> osImages = Lists.newArrayList();
> +
> +      getImagesFromPublisher("Microsoft.WindowsAzure.Compute", osImages, location);
> +      getImagesFromPublisher("MicrosoftWindowsServer", osImages, location);
> +      getImagesFromPublisher("Canonical", osImages, location);

@nacx This sounds really obvious, but let me just confirm my understanding. Are you thinking of something like:
`properties.put(IMAGE_PUBLISHERS, "Microsoft.WindowsAzure.Compute, MicrosoftWindowsServer, Canonical");` 

then in `getImagesFromPublisher`, loop thru `String[] publishers = properties.getProperty(IMAGE_PUBLISHERS).split(",");`

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65999960

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +   @Inject
> +   VMHardwareToHardware(@Memoized final Supplier<Set<? extends Location>> locations) {
> +      this.locations = locations;
> +   }
> +
> +   @Override
> +   public Hardware apply(VMHardware from) {
> +      final HardwareBuilder builder = new HardwareBuilder()
> +              .name(from.name)
> +              .id(from.name)
> +              .processors(ImmutableList.of(new Processor(from.numberOfCores, 2)))
> +              .ram(from.memoryInMB)
> +              .location(from.globallyAvailable ? null : FluentIterable.from(locations.get())
> +                      .firstMatch(LocationPredicates.idEquals(from.location))
> +                      .orNull());

This `orNull` would indicate a global hardware profile. Given how the method in the adapter is built, this should never happen, so I think we'd better fail if none is found?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65967714

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
> @@ -207,7 +208,7 @@ private void getImagesFromPublisher(String publisherName, List<VMImage> osImages
>  
>     private List<VMImage> listImagesByLocation(String location) {
>        final List<VMImage> osImages = Lists.newArrayList();
> -      String[] publishers = this.azureComputeConstants.azureImagePublishers().split(",");
> +      Iterable<String> publishers = Splitter.on(',').trimResults().omitEmptyStrings().split(this.azureComputeConstants.azureImagePublishers());
>        for (String publisher : publishers) {
>           publisher = publisher.trim();

I don't think you need to trim again

---
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/279/files/09ef3fda444dc993be925ba4a2907da6d0d2b544..06c34652e01ed4f1cfef461703348c1de220f0d6#r66223544

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +   private static final String SUSE = "SUSE";
> +
> +   private static final String SQL_SERVER = "SQL Server";
> +
> +   private static final String ORACLE_lINUX = "Oracle Linux";
> +
> +   private final Supplier<Set<? extends org.jclouds.domain.Location>> locations;
> +
> +   public static String encodeFieldsToUniqueId(VMImage imageReference){
> +      return (imageReference.globallyAvailable ? "global" : imageReference.location) + "/" + imageReference.publisher + "/" + imageReference.offer + "/" + imageReference.sku;
> +   }
> +
> +   public static String[] decodeFieldsFromUniqueId(final String id) {
> +      final String[] parts = checkNotNull(id, "id").split("/");
> +      return parts;

Just `return checkNotNull(id, "id").split("/");`

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65967848

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +               try {
> +                  Thread.sleep(30 * 1000);
> +               } catch (InterruptedException e) {
> +               }
> +               continue;
> +            }
> +            else
> +            {
> +               allRestarted = true;
> +            }
> +         }
> +      }
> +      assertTrue(allRestarted);
> +
> +      view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +   }

Also a subclass of the `BaseTemplateBuilderLiveTest` must be added. As usual, you can take the [DigitalOcean](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/test/java/org/jclouds/digitalocean2/compute/DigitalOcean2TemplateBuilderLiveTest.java) one as an example. Providers should have those tests passing, and try to avoid overriding the base class tests (as much as possible) as that is the contract to guarantee the provider is aligned with the jclouds interfaces and really portable.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65969887

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +               } else if (statuses.get(c).displayStatus().equals("VM stopped")) {
> +                  status = NodeMetadata.Status.SUSPENDED;
> +               }
> +               break;
> +            }
> +         }
> +      }
> +
> +      builder.status(status);
> +
> +      Credentials credentials = credentialStore.get("node#" + from.deployment.name());
> +      if (credentials != null && credentials.identity.equals(JCLOUDS_DEFAULT_USERNAME)) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, credentials.credential);
> +      } else if (credentials == null) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, DEFAULT_LOGIN_PASSWORD);
> +      }

@jtjk Can you 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65984306

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ilkka Salento <no...@github.com>.
> +
> +   @Inject
> +   VMHardwareToHardware(@Memoized final Supplier<Set<? extends Location>> locations) {
> +      this.locations = locations;
> +   }
> +
> +   @Override
> +   public Hardware apply(VMHardware from) {
> +      final HardwareBuilder builder = new HardwareBuilder()
> +              .name(from.name)
> +              .id(from.name)
> +              .processors(ImmutableList.of(new Processor(from.numberOfCores, 2)))
> +              .ram(from.memoryInMB)
> +              .location(from.globallyAvailable ? null : FluentIterable.from(locations.get())
> +                      .firstMatch(LocationPredicates.idEquals(from.location))
> +                      .orNull());

Using get() instead of orNull should throw  IllegalStateException in case location is not in the list ( which shouldn't happen) 
https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Optional.html#get()

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66023925

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +   }
> +
> +   private List<PublicIPAddress> getIPAddresses(Deployment deployment) {
> +      List<PublicIPAddress> list = new ArrayList<PublicIPAddress>();
> +      String resourceGroup = getResourceGroupFromId(deployment.id());
> +
> +      if (deployment.properties() != null && deployment.properties().dependencies() != null) {
> +         List<Deployment.Dependency> dependencies = deployment.properties().dependencies();
> +         for (int d = 0; d < dependencies.size(); d++) {
> +            if (dependencies.get(d).resourceType().equals("Microsoft.Network/networkInterfaces")) {
> +               List<Deployment.Dependency> dependsOn = dependencies.get(d).dependsOn();
> +               for (int e = 0; e < dependsOn.size(); e++) {
> +                  if (dependsOn.get(e).resourceType().equals("Microsoft.Network/publicIPAddresses")) {
> +                     String resourceName = dependsOn.get(e).resourceName();
> +                     PublicIPAddress ip = api.getPublicIPAddressApi(resourceGroup).get(resourceName);
> +                     list.add(ip);

Should we break here or continue iterating?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65965698

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Andrea Turli <no...@github.com>.
cool!

---
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/279#issuecomment-224316758

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +         return tcpRuleFormatProperty;
> +      }
> +
> +      public String tcpRuleRegexp() {
> +         return tcpRuleRegexpProperty;
> +      }
> +   }
> +
> +   @Provides
> +   @Named("azureGroupId")
> +   protected String getGroupId() {
> +      String group =  System.getProperty("test.azurecompute-arm.groupname");
> +      if (group == null)
> +         group = "jCloudsGroup";
> +      return group;
> +   }

Remove this test code from the provider code and configure the default value in the azure api metadata. The value can be overridden in the tests, if needed, just like any other property.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65966326

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

Thanks @andreaturli Have few questions about this. Are you available on IRC?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66093569

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Janne Koskinen <no...@github.com>.
> +               } else if (statuses.get(c).displayStatus().equals("VM stopped")) {
> +                  status = NodeMetadata.Status.SUSPENDED;
> +               }
> +               break;
> +            }
> +         }
> +      }
> +
> +      builder.status(status);
> +
> +      Credentials credentials = credentialStore.get("node#" + from.deployment.name());
> +      if (credentials != null && credentials.identity.equals(JCLOUDS_DEFAULT_USERNAME)) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, credentials.credential);
> +      } else if (credentials == null) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, DEFAULT_LOGIN_PASSWORD);
> +      }

I can't remember all the details now but we needed to do this. At least jClouds default username&password are not suitable with Azure. For example Azure does not allow to use "root" username.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66064594

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
Closed #279.

---
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/279#event-696453760

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +               } else if (statuses.get(c).displayStatus().equals("VM stopped")) {
> +                  status = NodeMetadata.Status.SUSPENDED;
> +               }
> +               break;
> +            }
> +         }
> +      }
> +
> +      builder.status(status);
> +
> +      Credentials credentials = credentialStore.get("node#" + from.deployment.name());
> +      if (credentials != null && credentials.identity.equals(JCLOUDS_DEFAULT_USERNAME)) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, credentials.credential);
> +      } else if (credentials == null) {
> +         credentials = new Credentials(AZURE_DEFAULT_USERNAME, DEFAULT_LOGIN_PASSWORD);
> +      }

So, the question here is "what happens if a user sets an invalid username when configuring the TemplateOptions?"
* If the deployment fails, then it is OK.
* If the deployment succeeds, then which username does the VM get?

In any case, jclouds has a mechanism to provide "default" credentials for images. Those default image credentials could be populated to the node if the user does not explicitly set ones.
Instead of setting the credentials constants in the DeploymentTemplateBuilder, you could configurate jclouds to populate them as default image credentials, then they could be accessed from a template with just `tempalte.getDefaultCredentials()` and be read by the DeploymentTemplateBuilder to provide the fallback credentials.

Configuring the default credentials for images just requires to set the `ComputeServiceProperties.IMAGE_LOGIN_USER` property to a sane default value for the username, or to a colon-delimited form providing also a default password: `<username>:<password>`.

If you configure that property in the metadata and read it in the deployment template builder to set it if the user does not explicitly set the credentials, then you could remove this code and assume the node has proper credentials. (This will however rely on the answer to the second point above).

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66075424

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +      DeploymentBody deploymentTemplateBody =  deploymentTemplateBuilder.getDeploymentTemplate();
> +
> +      DeploymentProperties properties = DeploymentProperties.create(deploymentTemplateBody);
> +
> +      final String deploymentTemplate = UrlEscapers.urlFormParameterEscaper().escape(deploymentTemplateBuilder.getDeploymentTemplateJson(properties));
> +
> +      logger.debug("Deployment created with name: %s", name);
> +
> +      final Set<VMDeployment> deployments = Sets.newHashSet();
> +
> +      ResourceGroupApi resourceGroupApi = api.getResourceGroupApi();
> +      ResourceGroup resourceGroup = resourceGroupApi.get(azureGroup);
> +      final String resourceGroupName;
> +
> +      if (resourceGroup == null){

Race conditions will appear here when deploying N nodes, as jclouds will call this method concurrently. All "shared" resources should be created *before* creating the actual nodes. This can be done by subclassing the `CreateNodesWithGroupEncodedIntoNameThenAddToSet` class and overriding its `execute` method. Look at how [DigitalOcean](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/compute/strategy/CreateKeyPairsThenCreateNodes.java) creates the key pairs before creating the nodes and how [it binds that strategy](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/compute/config/DigitalOcean2ComputeServiceContextModule.java#L92) to the Guice context.
Moving this logic there will eliminate the race condition and the code here will be able to assume the resource group always 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65964059

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Ignasi Barrera <no...@github.com>.
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.azurecompute.arm.domain;
> +
> +public class VMImage {

Use google auto for internal classes 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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65968245

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +         public boolean apply(final String name) {
> +            runningNumber++;
> +
> +            Deployment deployment = api.getDeploymentApi(resourceGroupName).create(name, deploymentTemplate);
> +
> +            if (deployment != null) {
> +               VMDeployment vmDeployment = new VMDeployment();
> +               vmDeployment.deployment = deployment;
> +               deployments.add(vmDeployment);
> +            } else {
> +               logger.debug("Failed to create deployment!");
> +            }
> +            return !deployments.isEmpty();
> +         }
> +      }, azureComputeConstants.operationTimeout(), 1, SECONDS).apply(name)) {
> +         final String illegalStateExceptionMessage = format("Deployment %s was not created within %sms so it will be destroyed.",

@andreaturli The `delete` operation of a template deployment does not take care of deletion of all the resources created as part of the deployment. When you click on "Delete" from the Azure portal, this is the message displayed:

> Delete deployment
Are you sure you want to delete this deployment? This will only delete the deployment history but will leave the resources and the resource group unaffected.

From MSDN document, here is more information on what the `delete` operation does:

> A template deployment that is currently running cannot be deleted. Deleting a template deployment removes the associated deployment operations. Deleting a template deployment does not affect the state of the resource group.

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r65910874

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm compute service (#279)

Posted by Rita Zhang <no...@github.com>.
> +               try {
> +                  Thread.sleep(30 * 1000);
> +               } catch (InterruptedException e) {
> +               }
> +               continue;
> +            }
> +            else
> +            {
> +               allRestarted = true;
> +            }
> +         }
> +      }
> +      assertTrue(allRestarted);
> +
> +      view.getComputeService().destroyNodesMatching(inGroup(groupName));
> +   }

@nacx Since we have few more PRs that depend on this to be merged, can we work on these two tests as another PR after this one gets merged as is?

---
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/279/files/4994250a3c7025efad0deeed0a72e30f77af856a#r66018309