Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-23 Thread Rita Zhang 
Closed #267.

---
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/267#event-669204504

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-20 Thread Andrea Turli 
> +  for (Deployment d : deployments){
> + VMDeployment vmDeployment = new VMDeployment();
> + vmDeployment.deployment = d;
> + vmDeployment.vm = 
> api.getVirtualMachineApi(getGroupId()).getInstanceDetails(d.name());
> + List list = getIPAddresses(d);
> + vmDeployment.ipAddressList = list;
> + vmDeployments.add(vmDeployment);
> +  }
> +  return vmDeployments;
> +   }
> +
> +   @Override
> +   public Iterable listNodesByIds(final Iterable ids) {
> +  System.out.println("listNodesByIds");
> +  for (String str : ids) {
> + System.out.println(str);

Remove

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r64097149

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-20 Thread Ignasi Barrera 
> + System.out.println(str);
> +  }
> +  return Iterables.filter(listNodes(), new Predicate() {
> + @Override
> + public boolean apply(final VMDeployment input) {
> +return Iterables.contains(ids, input.deployment.name());
> + }
> +  });
> +   }
> +
> +   private String getGroupId() {
> +  String group =  System.getProperty("test.azurecompute-arm.groupname");
> +  if (group == null)
> + group = "jCloudsGroup";
> +  return group;
> +   }

Thanks for the update. That's why I'm asking. There are two direct approaches 
here and we have to chose one:

1. Configure a resource group when creating the context (but provide a default 
resource group in case users don't configure it), and let jclouds only consider 
that resource group.
2. Assimilate a jclouds group to an Azure resource group.

**Option 1** is more straightforward *but* requires users configuring a 
specific group. It would limit the users experience of the provider "out of the 
box" when using it to manage existing noes in an account. What if users want to 
manage nodes that already were there *before* they started using jclouds? What 
if nodes are in different resource groups? (Is this something option 2 could 
manage?).

**Option 2** would better align the provider with the other ones. When calling 
"listNodes" users expect to be returned all the nodes in all locations. 
Returning just the ones in a configured resource group is very particular to 
Azure.

I tend to prefer Option 2, because the method in the compute service would be 
better aligned with how jclouds behaves in the rest of providers, and would 
allow users to use jclouds on existing accounts out of the box. However, this 
assumes that a resource group is a good match fort a jclouds group (which I 
think it is, with the little knowledge I have of the ARM API).

Thoughts? I'm happy to discuss pros, cons, or different approaches, but getting 
the model right is crucial to implement the provider. Also, feel free to move 
this discussion to the `dev@` mailing list if you prefer a discussion about 
this 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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r64006937

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-20 Thread Janne Koskinen 
> +   VMSizeToHardware vmSizeToHardware, Map 
> credentialStore) {
> +
> +  this.nodeNamingConvention = namingConvention.createWithoutPrefix();
> +  this.locations = checkNotNull(locations, "locations");
> +  this.imageReferenceToImage = imageReferenceToImage;
> +  this.vmSizeToHardware = vmSizeToHardware;
> +  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());

Backend is using name as id.

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r64006142

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-20 Thread Janne Koskinen 
> +   GroupNamingConvention.Factory namingConvention, 
> ImageReferenceToImage imageReferenceToImage,
> +   VMSizeToHardware vmSizeToHardware, Map 
> credentialStore) {
> +
> +  this.nodeNamingConvention = namingConvention.createWithoutPrefix();
> +  this.locations = checkNotNull(locations, "locations");
> +  this.imageReferenceToImage = imageReferenceToImage;
> +  this.vmSizeToHardware = vmSizeToHardware;
> +  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());

That is the only id. It would make sense to add group name there, but I am not 
sure. I will check that.

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r64006067

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-20 Thread Janne Koskinen 
> + System.out.println(str);
> +  }
> +  return Iterables.filter(listNodes(), new Predicate() {
> + @Override
> + public boolean apply(final VMDeployment input) {
> +return Iterables.contains(ids, input.deployment.name());
> + }
> +  });
> +   }
> +
> +   private String getGroupId() {
> +  String group =  System.getProperty("test.azurecompute-arm.groupname");
> +  if (group == null)
> + group = "jCloudsGroup";
> +  return group;
> +   }

Hi, My old comment from our slack below. IMHO it is good to have it 
jCloudsGroup as default and let users to config that if they want. If we use 
jClouds groups as resource groups, some of the jClouds-features are not working 
as we can work in one resource group per time.

Example: if we create 10 VMs and we name them "machine", there will be jClouds 
group called "machine" and VMs will be named like machine-e3e, machine-e3f, 
machine-e3g etc

[9:28] 
if we then create another 10 VMs and name them machine2, there will be new 
group machine2 etc

[9:29] 
if we use this machine2 as azure resource group name, the jClouds API will be 
working incorrectly

[9:29] 
Because, there is function that we can use to run script in all machines 
matching some regex

[9:30] 
if we say "runScriptInAllMachinesMatching("machine")", we should run that 
script in machine-group and machine2-group.

[9:31] 
and basically jClouds will then call our getAllNodes once

[9:31] 
and we have implemented getAllNodes so that it will return nodes only from one 
resourceGroup



---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r64004959

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Rita Zhang 
@nacx  No worries! 

> I think splitting the PR will help you focus and also advance in other areas 
> while the compute service is a WIP. 

That makes sense to me. I will go ahead and split this PR into two as you 
suggested and address some of these comments for the 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/267#issuecomment-220481422

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> bump We have folks who are waiting to test this in few days. Would be awesome 
> if we could get this merged soon since we have couple of more PRs left. Thank 
> you so much for all your feedback!!!

Apologies for the delay. I'm just back from ApacheCon in Vancouver and was 
still going through my backlog :)

I think with the current comments there are many things to think about and 
discuss. I think it would make sense to split this into two pull requests: one 
with all the APIs, and one with the compute service implementation and the 
transformation functions. The main reason is:

* APIs will be merged sooner.
* Implementing the compute service will bring some design discussions onto the 
table (as seen). How to design user access to VMs, how to match jclouds 
entities to azure ones, etc.
* We consider the compute service properly implemented once there is a test 
that extends the `BaseComputeServiceLiveTest` (have a look at the 
[digitalocean](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/test/java/org/jclouds/digitalocean2/compute/DigitalOcean2ComputeServiceLiveTest.java)
 one for an example) and one that extends the `BaseTemplateBuilderLiveTest` 
(again, see the 
[digitalocean](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/test/java/org/jclouds/digitalocean2/compute/DigitalOcean2TemplateBuilderLiveTest.java)
 one). These tests *are the contract* for the compute service and must be 
passing (with little and minor modifications, if you look at the examples 
you'll see they just add things and override just very few things) to show that 
the jclouds expected functionality is covered.

I think splitting the PR will help you focus and also advance in other areas 
while the compute service is a WIP. It will also help to develop it step by 
step: we can perhaps merge first just the compute service (with a proper design 
and entity mappings) and later on merge the tests and the fixes required to the 
model/impl to make them pass and meet the jclouds contract. WDYT?

Look forward to seeing all this stuff 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/267#issuecomment-220476220

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +  this.locations = checkNotNull(locations, "locations");
> +  this.imageReferenceToImage = imageReferenceToImage;
> +  this.vmSizeToHardware = vmSizeToHardware;
> +  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("-");

Take care. When using the group naming convention the separator is configurable 
via properties. If you are splitting such generated names, then consider using 
the group naming convention methods to get the group 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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63967447

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +   VMSizeToHardware vmSizeToHardware, Map 
> credentialStore) {
> +
> +  this.nodeNamingConvention = namingConvention.createWithoutPrefix();
> +  this.locations = checkNotNull(locations, "locations");
> +  this.imageReferenceToImage = imageReferenceToImage;
> +  this.vmSizeToHardware = vmSizeToHardware;
> +  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());

This value should be the real "id" in the backend. It looks like the value 
should be `deployment.id()`? 

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63967331

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +   GroupNamingConvention.Factory namingConvention, 
> ImageReferenceToImage imageReferenceToImage,
> +   VMSizeToHardware vmSizeToHardware, Map 
> credentialStore) {
> +
> +  this.nodeNamingConvention = namingConvention.createWithoutPrefix();
> +  this.locations = checkNotNull(locations, "locations");
> +  this.imageReferenceToImage = imageReferenceToImage;
> +  this.vmSizeToHardware = vmSizeToHardware;
> +  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());

This is the "id" jclouds will use as the main id (used to call `getNode`, etc). 
Can this value uniquely identify the node across all locations?

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63967239

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +
> +   private final ImageReferenceToImage imageReferenceToImage;
> +
> +   private final VMSizeToHardware vmSizeToHardware;
> +
> +   private final Map credentialStore;
> +
> +   @Inject
> +   DeploymentToNodeMetadata(
> +   AzureComputeApi api,
> +   @Memoized Supplier locations,
> +   GroupNamingConvention.Factory namingConvention, 
> ImageReferenceToImage imageReferenceToImage,
> +   VMSizeToHardware vmSizeToHardware, Map 
> credentialStore) {
> +
> +  this.nodeNamingConvention = namingConvention.createWithoutPrefix();
> +  this.locations = checkNotNull(locations, "locations");

Redundant check. The injector already checks if it is 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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63967148

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +  List vmDeployments = new ArrayList();
> +
> +  for (Deployment d : deployments){
> + VMDeployment vmDeployment = new VMDeployment();
> + vmDeployment.deployment = d;
> + vmDeployment.vm = 
> api.getVirtualMachineApi(getGroupId()).getInstanceDetails(d.name());
> + List list = getIPAddresses(d);
> + vmDeployment.ipAddressList = list;
> + vmDeployments.add(vmDeployment);
> +  }
> +  return vmDeployments;
> +   }
> +
> +   @Override
> +   public Iterable listNodesByIds(final Iterable ids) {
> +  System.out.println("listNodesByIds");

Remove

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965857

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +   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);
> +  }
> +   }
> +}
> + }
> +  }
> +  return list;
> +   }
> +
> +   @Override
> +   public Iterable listNodes() {
> +  System.out.println("listNodes");

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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965784

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> + if (jobDone) {
> +// Delete storage account
> +
> api.getStorageAccountApi(getGroupId()).delete(storageAccountName);
> +
> +// Delete NIC
> +uri = api.getNetworkInterfaceCardApi(getGroupId()).delete(id + 
> "nic");
> +if (uri != null){
> +   jobDone = Predicates2.retry(new Predicate() {
> +  @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);
> +   if (jobDone) {
> +  // Delete public ip
> +  api.getPublicIPAddressApi(getGroupId()).delete(id + 
> "publicip");

Does it make sense to do a "best effort delete" and try to delete the 
deployment and network even if this call fails?

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965750

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +   }
> +
> +   @Override
> +   public ImageReference getImage(final String id) {
> +  Iterable images = listImages();
> +  for (ImageReference image : images) {
> + if (id.contains(image.offer()) && id.contains(image.sku())) {
> +return image;
> + }
> +  }
> +  return null;
> +   }
> +
> +   private String getLocation() {
> +  return "eastasia"; // TODO: get location
> +   }

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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965349

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> + }
> +  }
> +   }
> +
> +   @Override
> +   public Iterable listImages() {
> +  final List osImages = Lists.newArrayList();
> +  getImagesFromPublisher("Microsoft.WindowsAzure.Compute", osImages);
> +  getImagesFromPublisher("MicrosoftWindowsServer", osImages);
> +  getImagesFromPublisher("Canonical", osImages);
> +  return osImages;
> +   }
> +
> +   @Override
> +   public ImageReference getImage(final String id) {
> +  Iterable images = listImages();

Listing images is often an expensive operation. Isn't there an api to get a 
single image?

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965315

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +  OSImageApi osImageApi = api.getOSImageApi(getLocation());
> +  Iterable offerList = osImageApi.listOffers(publisherName);
> +  for (Offer offer : offerList) {
> + Iterable skuList = osImageApi.listSKUs(publisherName, 
> offer.name());
> + for (SKU sku : skuList) {
> +osImagesRef.add(ImageReference.create(publisherName, 
> offer.name(), sku.name(), null));
> + }
> +  }
> +   }
> +
> +   @Override
> +   public Iterable listImages() {
> +  final List osImages = Lists.newArrayList();
> +  getImagesFromPublisher("Microsoft.WindowsAzure.Compute", osImages);
> +  getImagesFromPublisher("MicrosoftWindowsServer", osImages);
> +  getImagesFromPublisher("Canonical", osImages);

It should be better to provide a property with the comma separated values of 
publishers. The api metadata should set the default value to these three 
elements, but this way we allow users to customize the list of publishers they 
want to use, when creating the context.

The same contract for locations applies here. After being transformed by the 
transformation function, global images must not have the location set, and 
images that only exist in a particular location must inform it.

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965264

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> + name, azureComputeConstants.operationTimeout());
> + logger.warn(illegalStateExceptionMessage);
> +
> + throw new IllegalStateException(illegalStateExceptionMessage);
> +  }
> +
> +  final VMDeployment deployment = deployments.iterator().next();
> +
> +
> +  return new NodeAndInitialCredentials(deployment, name,
> +  
> LoginCredentials.builder().user(loginUser).identity(loginUser).password(loginPassword).authenticateSudo(true).build());
> +   }
> +
> +   @Override
> +   public Iterable listHardwareProfiles() {
> +  return api.getVMSizeApi(getLocation()).list();

This must return the hardware profiles for *all* locations, and how it works 
must be coded in conjunction with the function that transforms the returned 
elements into a jclouds `Hardware`

* If a hardawre profile does not exist in all locations, then when transforming 
it, the `location` field must be set.
* if a hardware profile is global and exists in all locations, then when 
transforming it, the `location` field must be left null.

Take this into account so you return here something that gives the 
transformation function enough context to implement this contract. You can 
create just an intermediate object here and use it to represent hardware 
profiles. This object does not have to be an Azure API object, but any object 
that is convenient; it will never be exposed to users.

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63965010

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +}
> +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.",
> + name, azureComputeConstants.operationTimeout());
> + logger.warn(illegalStateExceptionMessage);
> +
> + throw new IllegalStateException(illegalStateExceptionMessage);
> +  }
> +
> +  final VMDeployment deployment = deployments.iterator().next();
> +
> +
> +  return new NodeAndInitialCredentials(deployment, name,
> +  
> LoginCredentials.builder().user(loginUser).identity(loginUser).password(loginPassword).authenticateSudo(true).build());

Also, authenticate sudo should be configured based on the value it has 
configured in the template options?

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63964676

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +  final String deploymentTemplate = 
> UrlEscapers.urlFormParameterEscaper().escape(json.toJson(properties));
> +
> +  logger.debug("Deployment created with name: %s", name);
> +
> +  final Set deployments = Sets.newHashSet();
> +  if (!retry(new Predicate() {
> + @Override
> + public boolean apply(final String name) {
> +runningNumber++;
> +
> +
> +ResourceGroupApi resourceGroupApi = api.getResourceGroupApi();
> +ResourceGroup resourceGroup = resourceGroupApi.get(getGroupId());
> +String resourceGroupName;
> +
> +if (resourceGroup == null){

Race conditions may appear here! When calling 
`computeService.createNodesInGroup` to create more than one node, jclouds will 
call this method concurrently.
If you need to perform some operations that affect *all* nodes being deployed, 
and perform them just once (such as creating a group, creating the networks or 
security groups indicated in the options, etc), then the right approach is to 
override the `CreateNodesInGroupThenAddToSet` strategy to do that. Take the 
[DigitalOcean 
one](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/compute/config/DigitalOcean2ComputeServiceContextModule.java#L92)
 as an example. That strategy is invoked only once, just before spawning N 
threads to create the nodes concurrently.

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63964298

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +  Gson gson = new GsonBuilder().disableHtmlEscaping().create();
> +  org.jclouds.json.Json json = new GsonWrapper(gson);
> +
> +  final String deploymentTemplate = 
> UrlEscapers.urlFormParameterEscaper().escape(json.toJson(properties));
> +
> +  logger.debug("Deployment created with name: %s", name);
> +
> +  final Set deployments = Sets.newHashSet();
> +  if (!retry(new Predicate() {
> + @Override
> + public boolean apply(final String name) {
> +runningNumber++;
> +
> +
> +ResourceGroupApi resourceGroupApi = api.getResourceGroupApi();
> +ResourceGroup resourceGroup = resourceGroupApi.get(getGroupId());

The group id and the api class could be assigned outside the predicate, to 
compute them only once.

---
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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63964025

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Ignasi Barrera 
> +import org.jclouds.domain.LoginCredentials;
> +import org.jclouds.json.internal.GsonWrapper;
> +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 {

This should extend the `BaseComputeService` instead of directly implement the 
interface. That base class performs many needed operations such as maintaining 
the credential store, polling the status of nodes, and much 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/267/files/bae5d6dfcdb26e31057db4551c599683a31b#r63963424

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-19 Thread Rita Zhang 
_bump_ @nacx We have folks who are waiting to test this in few days. Would be 
awesome if we could get this merged soon since we have couple of more PRs left. 
Thank you so much for all your feedback!!!

---
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/267#issuecomment-220456609

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-18 Thread Rita Zhang 
@nacx Rebuilt successfully. Ready for 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/267#issuecomment-220058766

Re: [jclouds/jclouds-labs] JCLOUDS-664 Azurecompute-arm DeploymentApi OSImageApi VMSizeApi Bindings (#267)

2016-05-18 Thread Ignasi Barrera 
Just triggering a new build now that our CI builds are fixed: rebuild please

---
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/267#issuecomment-220032483