Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Closed #145.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#event-344184478

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Pushed to 
[master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/17a838b2) 
and 
[1.9.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/d07fcb17). 
Thanks for all the effort @devcsrj!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-117170013

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

>Use #getNetworks from TemplateOptions; Fallback to default value if invalid.

Can a server be attached to more than one network?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-117120899

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Re-squashed commits, with changes including:
- Use 
[ComputeServiceUtils](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-1a27cebd5920714ae4c207026edd5255R153)
 .
- Updated README.md. 
[1](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-60534573ad8a36041d9c4ad20d2927f2R48),
 
[2](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-60534573ad8a36041d9c4ad20d2927f2R62)
- 
[Use](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-1a27cebd5920714ae4c207026edd5255R145)
 `#getNetworks` from TemplateOptions; Fallback to default value if invalid.

This PR doesn't have a `ProfitBricksTemplateOptions`, is this required? Or will 
this do for now (and maybe be updated in a future PR)?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-116914838

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> + .imageNameMatches( "" )
> + .options( new TemplateOptions()
> + .overrideLoginUser( "root" ) // unless you 
> changed the user
> + .overrideLoginPassword( "" ))
> + // more options, as you need
> + .build();
> + 
> +compute.createNodesInGroup( "cluster1", 1, template );
> +```
> +> If no `locationId` is specified in the template, jclouds will look for a 
> `DataCenter` that is of same scope as the `Image`.
> +
> +
> +## Limitations
> +
> +1. There's no direct way of specifying arbitrary number of cores, RAM size, 
> and storage size via the compute interface, at least until after 
> [JCLOUDS-482](https://issues.apache.org/jira/browse/JCLOUDS-482) is resolved. 
> The adapter uses a predefined list hardware profiles instead.
> +2. All provisioned nodes are connected at LAN 1 of a datacenter.

You can have a look at 
[openstack-nova](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java#L119-L121).

The network ids will come as a String and it is up to the adapter to use those 
values. The id is provider-specific, so you can assume they are already the IDs 
of the LANs or whatever makes sense int he provider to attach the nodes to 
those networks.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33446049

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +...
> +Changing password for root.
> +(current) UNIX password: 
> +Enter new UNIX password: 
> +Retype new UNIX password: 
> +~ root@ubuntu:~# exit
> +
> +```
> +
> +- Go back to the DCD, and *make a snapshot* of the storage. Put a 
> descriptive name.
> +- Configure jclouds to use this *snapshot*.
> +
> +```java 
> +Template template = compute.templateBuilder()
> + .imageNameMatches( "" )
> + .options( new TemplateOptions()

No, just to substitute the "new". Don't encourage users to build a new template 
options object; it should be better retrieved from the ComputeService, as it 
will provide the provider-specific implementation allowing users to use the 
`.as(ProfitBricksTempalteOptions.class)` when needed.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33445746

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> + .imageNameMatches( "" )
> + .options( new TemplateOptions()
> + .overrideLoginUser( "root" ) // unless you 
> changed the user
> + .overrideLoginPassword( "" ))
> + // more options, as you need
> + .build();
> + 
> +compute.createNodesInGroup( "cluster1", 1, template );
> +```
> +> If no `locationId` is specified in the template, jclouds will look for a 
> `DataCenter` that is of same scope as the `Image`.
> +
> +
> +## Limitations
> +
> +1. There's no direct way of specifying arbitrary number of cores, RAM size, 
> and storage size via the compute interface, at least until after 
> [JCLOUDS-482](https://issues.apache.org/jira/browse/JCLOUDS-482) is resolved. 
> The adapter uses a predefined list hardware profiles instead.
> +2. All provisioned nodes are connected at LAN 1 of a datacenter.

Oh. Is there another provider I could follow for this? I think I saw something 
like `DockerTemplateOptions` in docker, is that it?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33445259

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> +...
> +Changing password for root.
> +(current) UNIX password: 
> +Enter new UNIX password: 
> +Retype new UNIX password: 
> +~ root@ubuntu:~# exit
> +
> +```
> +
> +- Go back to the DCD, and *make a snapshot* of the storage. Put a 
> descriptive name.
> +- Configure jclouds to use this *snapshot*.
> +
> +```java 
> +Template template = compute.templateBuilder()
> + .imageNameMatches( "" )
> + .options( new TemplateOptions()

You mean, a separate line like this?
```
compute.templateOptions()
.overrideLoginUser( "root" )
.overrideLoginPassword( "password" );

Template template = compute.templateBuilder().imageNameMatches("---").build();
```

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33445194

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> + .imageNameMatches( "" )
> + .options( new TemplateOptions()
> + .overrideLoginUser( "root" ) // unless you 
> changed the user
> + .overrideLoginPassword( "" ))
> + // more options, as you need
> + .build();
> + 
> +compute.createNodesInGroup( "cluster1", 1, template );
> +```
> +> If no `locationId` is specified in the template, jclouds will look for a 
> `DataCenter` that is of same scope as the `Image`.
> +
> +
> +## Limitations
> +
> +1. There's no direct way of specifying arbitrary number of cores, RAM size, 
> and storage size via the compute interface, at least until after 
> [JCLOUDS-482](https://issues.apache.org/jira/browse/JCLOUDS-482) is resolved. 
> The adapter uses a predefined list hardware profiles instead.
> +2. All provisioned nodes are connected at LAN 1 of a datacenter.

TBD in future PRs (not now). The template options allow to specify a set of 
"network ids". Perhaps that is the place to allow users to specify to which 
LANs the servers should be attached. The adapter should take into account that 
option.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33444666

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +...
> +Changing password for root.
> +(current) UNIX password: 
> +Enter new UNIX password: 
> +Retype new UNIX password: 
> +~ root@ubuntu:~# exit
> +
> +```
> +
> +- Go back to the DCD, and *make a snapshot* of the storage. Put a 
> descriptive name.
> +- Configure jclouds to use this *snapshot*.
> +
> +```java 
> +Template template = compute.templateBuilder()
> + .imageNameMatches( "" )
> + .options( new TemplateOptions()

[minor] chante to `compute.templateOptions()`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33444613

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Just one very minor comment, otherwse LGTM.

One important thing to remember: IIRC the default templates provided by 
ProfitBricks weren't accessible via SSH, right? People have to create an image 
in their account to make it usable (or at least to make it usable with all the 
features in the ComputeService) by jclouds. If that is the case, you should add 
a README file explaining a bit how the provider works, and the steps that need 
to be done in order to be able to use the provider.

Fantastic job, @devcsrj!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115628386

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +   public Object get() {
> +  return api.storageApi().createStorage(request);
> +   }
> +}));
> +
> +storageIds.add(storageId);
> +logger.trace(">> provisioning complete for storage. returned 
> id='%s'", storageId);
> + } catch (Exception ex) {
> +if (i - 1 == 1) // if first storage (one with image) 
> provisioning fails; stop method
> +   throw Throwables.propagate(ex);
> +logger.warn(ex, ">> failed to provision storage. skipping..");
> + }
> +
> +  Double cores = 0d;
> +  for (Processor processor : hardware.getProcessors())
> + cores += processor.getCores();

[minor] Use the 
[ComputeServiceUtils](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/util/ComputeServiceUtils.java#L116-L121)
 instead.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r33343455

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

rebuild please

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115538443

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Alright! With the template equals assertion fixed, the 
`BaseTemplateBuilderLiveTest` passes perfectly. I squashed the commits again, 
with the latest change that include:
- 
[Removed](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-d60e9024736ca1880e1eb4a045763065R69)
 `ProfitBricksTemplateBuilder` altogether - the default behavior was sufficient
- Used `DateService` instead of `DateCodecFactory` - 
[resolved](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-11ea89320c87f906e05816c31c5773a1R87)
 the unparseable date issue mentioned yesterday.

P.S.: There's a compilation error in the *jdbc* module. I was wondering if I 
fix it in this PR, or create a separate one. I chose the latter 
[PR181](https://github.com/jclouds/jclouds-labs/pull/181).

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115505673

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

I think the assertion in that test is not accurate. Given 
[this](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java#L689-L690),
 the location of the TemplateBuilder is set to the most concrete location. If 
the belongs to a location more concrete than the one specified in the ID, the 
location is changed.

The test does not take that into account, but given how the TemplateBuilder 
works, it is possible that the selected image has the same location, or a 
location "contained" in the specified location.

The way to go would be to override that method and replace the assertion by N 
assertions doing the same, but when asserting the location, check that the 
location is the same or contained in the given location. Even better, could you 
do this fix in a PR in the main repo to fix the test in the base class?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115179545

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> The OnlyLocationOrFirstZone already exists in jclouds and will only take into 
> account zone locations.

AH. Because of this, I could remove the `ProfitBricksTemplateBuilder` 
altogether. :+1: 

One last thing though: the 
[BaseTemplateBuilderLiveTest.testTemplateBuilderWithImageIdSpecified](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseTemplateBuilderLiveTest.java#L72)
 fails, with the result of:

```
Failed tests: 
  
ProfitBricksTemplateBuilderLiveTest>BaseTemplateBuilderLiveTest.testTemplateBuilderWithImageIdSpecified:159
 expected [{image={id=a380aec9-080b-11e5-b78a-52540066fee9, 
providerId=a380aec9-080b-11e5-b78a-52540066fee9, 
name=Ubuntu-12.04-LTS-server-2015-06-01, location={scope=REGION, id=us/lasdev, 
description=USA Developer cluster, parent=profitbricks}, os={family=ubuntu, 
version=12.04, description=ubuntu, is64Bit=true}, status=AVAILABLE, 
loginUser=root}, hardware={id=cpu=1,ram=1024,disk=10.00, 
providerId=cpu=1,ram=1024,disk=10.00, name=cpu=1,ram=1024,disk=10.00, 
processors=[{cores=1.0, speed=1.0}], ram=1024, volumes=[{type=LOCAL, size=10.0, 
bootDevice=true, durable=true}], hypervisor=kvm, supportsImage=ALWAYS_TRUE}, 
location={scope=ZONE, id=abc433ce-6d21-47c9-a7eb-5342fde68dbf, 
description=DCtest001, parent=us/lasdev, metadata={version=2, 
state=AVAILABLE}}, options={}}] but found 
[{image={id=a380aec9-080b-11e5-b78a-52540066fee9, 
providerId=a380aec9-080b-11e5-b78a-5
 2540066f
 ee9, name=Ubuntu-12.04-LTS-server-2015-06-01, location={scope=REGION, 
id=us/lasdev, description=USA Developer cluster, parent=profitbricks}, 
os={family=ubuntu, version=12.04, description=ubuntu, is64Bit=true}, 
status=AVAILABLE, loginUser=root}, hardware={id=cpu=1,ram=1024,disk=10.00, 
providerId=cpu=1,ram=1024,disk=10.00, name=cpu=1,ram=1024,disk=10.00, 
processors=[{cores=1.0, speed=1.0}], ram=1024, volumes=[{type=LOCAL, size=10.0, 
bootDevice=true, durable=true}], hypervisor=kvm, supportsImage=ALWAYS_TRUE}, 
location={scope=REGION, id=us/lasdev, description=USA Developer cluster, 
parent=profitbricks}, options={}}]
```
The difference was that, the default template used a more specific location 
(ZONE), while the template with assigned image used the image location instead 
(REGION). What's the best way to approach this?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115174692

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

>The template produced has a location of type REGION (us/lasdev, us/las, 
>etc.,). ProfitBricks provisions a node to a datacenter though (ZONE). Then, 
>there's no method like TemplateBuilder#getLocationId() to check whether the 
>locationId was already set, so I was forced to parse again the property.

Then the right approach is to change the default implicit location supplier. 
Something like this in the compute service context module should work:

```java
bind(ImplicitLocationSupplier.class).to(OnlyLocationOrFirstZone.class).in(Scopes.SINGLETON);
```
The `OnlyLocationOrFirstZone` already exists in jclouds and will only take into 
account zone locations.

>P.S.: I removed the ProfitBricksTemplateBuilder#build(). It turns out, it 
>wasn't needed. Also I added this catch, since weirdly enough, it threw 
>Unparseable date: "2015-06-18T14:16:51+", even when the actual value 
>returned by the server is 2015-06-18T14:16:51Z.

Perhaps you have more control on how the date is parsed if you inject the 
`DateService` instead?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115162267

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

The reason for this override is because, when I do:
```
ComputeService compute = ContextBuilder.newBuilder("profitbricks")
.credentials("identity", "credential")
.buildView(ComputeServiceContext.class)
.getComputeService();
Template template = compute.templateBuilder().build();
```
The `template` produced has a location of type `REGION` (`us/lasdev`, `us/las`, 
etc.,). ProfitBricks provisions a node to a datacenter though (`ZONE`). Then, 
there's no method like `TemplateBuilder#getLocationId()` to check whether the 
locationId was already set, so I was *forced* to parse again the property.

Both `BaseTemplateBuilderLiveTest` and `BaseComputeServiceLiveTest` passes with 
or without this method, but then again I am passing a template, so the context 
uses the location of the image.

What'ya think?

P.S.: I removed the `ProfitBricksTemplateBuilder#build()`. It turns out, it 
wasn't needed. Also I added this 
[catch](https://github.com/devcsrj/jclouds-labs/commit/f4b1311c3cb4f4caf8b17c31e6cb6597d151bce9#diff-11ea89320c87f906e05816c31c5773a1R90),
 since weirdly enough, it threw `Unparseable date: "2015-06-18T14:16:51+"`, 
even when the actual value returned by the server is `2015-06-18T14:16:51Z`.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115143706

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Thanks @devcsrj!

>The part left I'm not sure of is my use of `@Named("DEFAULT2")` here. There 
>seems to be a prettier approach.

Having a look at the method, I don't really get why you need to override it 
(and I think you shouldn't). The TemplateBuilderSpec already considers the 
"locationId" property, so there is no need to provide a separate property to 
configure it to be taken into account to configure the default template (to be 
used if none is provided when calling the createNodes method).

Also, there is no need to explicitly returning the first Datacenter, as the 
tempalte builder will fallback to the [default 
location](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java#L700-L703)
 if none is provided. That default location supplier is configured by the 
[ImplicitLocationSupplier](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/location/suppliers/ImplicitLocationSupplier.java),
 which default implementation returns the first region or zone. 

The default jclouds behaviour should cover what you are trying to do by 
overriding this method, if I have understood it properly. Does this make sense?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-115007146

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi @nacx !

The PR's been rebased and squashed. :)
[Live test 
results](https://gist.github.com/devcsrj/828e5be44a54f5217cbd/b6055d1d6c84848154d0a0f94279121140ace03a)
 `Tests run: 25, Failures: 0, Errors: 0, Skipped: 0` at `1:01:56.334s`

P.S.: The part left I'm not sure of is my use of 
[@Named("DEFAULT2")](https://github.com/jclouds/jclouds-labs/commit/97aa7a2f7d268f167403ffe47c3edd5a2a444306#diff-d60e9024736ca1880e1eb4a045763065R128)
 here. There seems to be a prettier approach.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-113154706

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

>The live tests creates multiple contexts

True, there is an explicit tests in the base compute service test class that 
creates multiple contexts to make sure there is no shared state between them. 
Good to see you figured out what was going on. Having unit tests always helps 
(that's why I consistently ask for a good coverage).

Once the live tests pass, mind squashing the commits and rebasing to the latest 
master version (there are several conflicts)? I'll then do a final review and 
merge it.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-113080386

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi @nacx ! Sorry for the delay, I was preoccupied with other things.

- *Does it fail if you don't add it to the jclouds closer?* I tried commenting 
out the `closer.add(..)` line, but still threw the error.
- *"..not 100% sure that will be guice friendly."* Yeah it wasn't. Too bad. 
Kinda messy type casts.

But more importantly, because of the `ProvisioningManagerTest`, the exception 
was reproduced. It was because of the [executors 
map](https://github.com/devcsrj/jclouds-labs/commit/0e783857c09d94b3770583bb52a23fbee1d894ca#diff-c67d5d74a6059f235c249f2e9a00a176L47)
 being a static variable. The live tests creates multiple contexts; and 
afterwards closed. Since it's a static variable, the rest of the instances held 
a closed executor. Phew, you're a code wizard. \m/

Anyway, before I squash them (and fix conflicts), I'll re-run a live test in a 
few moments and re-post the latest results. :)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-113077243

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

@devcsrj I've seen a couple of things that are not OK in the Provisioning 
manager. I've done some minor changes and fixed some stuff, but haven't tried 
running the live tests. You can see the diff here: 
https://github.com/nacx/jclouds-labs/compare/199a90fd53526c8a6e7803ea51be157da77154c3...profitbricks

Things I've found:

* Using the `putIfAbsent` to put a new ExecutorService to the map, could leak 
executor services when the value was absent. They were created but not added.
* Once the manager was closed, one could still enqueue jobs, as the executors 
are created on demand.
* I've also changed the manager to be properly typed, but I'm not 100% sure 
that will be guice friendly :) Just discard that part if it does not work.

Could you try running the live tests with my changes?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-112718397

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

I can't see anything wrong in the configuration. Is it consistent that the 
tests suceed on `6d71bbb` and fail on `199a90f`?

Does it fail if you **don't** add it to the jclouds closer? That would indicate 
that the context could be being closed while there are still tasks being 
submitted or in the execution queue. Could that be the case?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-112383275

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Yo @nacx ! You're approach is. simply. too. awesome. +1 

So I refactored the adapter to follow your approach. It worked well on 
`6d71bbb`. On commit  `199a90f` however, re-running the live test throws a 
`java.util.concurrent.RejectedExecutionException` 
([Stacktrace](http://pastie.org/private/yxap1nx09xrmk3rkeaieg)).

I think I misconfigured the *provider* somehow. Notice anything wrong? 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-106697138

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

I've had a look at the adapter and looks good. The ProvisioningManager approach 
looks clean! A couple questions:

* When is the ProvisioningManager closed? I haven't seen a call to its close 
method and we need to make sure its executors are closed.

I'd suggest to take a slightly different approach, to bind the 
ProvisioningManager to the jclouds context lifecycle, but that would require 
some (minor) changes to make it better managed by Guice:

* Configure the ProvisioningManager in a *provider* method in the Compute 
module, so it can be added to the jclouds Closer. Have a look at the 
[ExecutorServiceModule](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/concurrent/config/ExecutorServiceModule.java#L135-L138)
 to see an example. This way it will be closed then the jclouds context is 
closed.
* Given that the ProvisioningJob is always about the datacenter, I would 
specialise it, and configure it with Guice so it gets the predicate already 
injected. That would leave the adapter code cleaner. To do that, you'll have to 
use the Guice assisted inject so you can provide the operation to run. It could 
be something like:

```java
public class ProvisioningJob implements Job {
   public interface Factory {
  ProvisioningJob create(String group, Supplier operation);
   }

   private final Predicate waitDataCenterUntilReady;
   private final String group;
   private final Supplier operation;
   
   @Inject ProvisioningJob(@Named(POLL_PREDICATE_DATACENTER) Predicate 
waitDataCenterUntilReady,
 @Assisted String group, @Assisted Supplier operation) {
  this.waitDataCenterUntilReady = waitDataCenterUntilReady;
  this.group = checkNotNull(group, "group cannot be null");
  this.operation = checkNotNull(operation, "operation cannot be null");
   }

   @Override
   public Object call() throws Exception {
  waitDataCenterUntilReady.apply(group);
  Object obj = operation.get();
  waitDataCenterUntilReady.apply(group);
  return obj;
   }
}
```
Then, you should configure it in the ComputeService module *configure* method 
as follows:
```java
install(new FactoryModuleBuilder().build(ProvisioningJob.Factory.class));
```
And finally, use it in the compute service adapter, by injecting the job 
factory:
```java
// Inject the job factory
private final ProvisioningJob.Factory jobFactory;
(...)
// Use it to schedule a new job
provisioningManager.provision(jobFactory.create(dataCenterId, new 
Supplier() {
   @Override public Object get() {
  return api.serverApi().createServer(serverRequest)
   })
);
```

It's just a suggestion to keep the job creation more self-contained and 
injectable, and to make the code a bit more concise.

Great job btw! I'll finish the review later today. Meanwhile, feel free to 
squash the commits and apply any changes you might have.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-106287652

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> @@ -36,7 +36,11 @@
>  FIXME
>  FIXME
>  1.3
> -
> +
> +locationId=45a8667e-edfe-4f64-8a4d-79d53c8fec1f,
> +imageNameMatches=jclouds-ubuntu14.10-template,
> +loginUser=root:asdfghjk
> +

Is this a public template anyone will be able to use, or is it a custom one in 
your account? If it is a private one and the ones provided by default by 
Profitbricks don't provide remote access, I'd add a README with the 
instructions to build a  *jclouds friendly* image and how to run the live tests 
using it.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r31222936

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

[Live test 
results.](https://gist.github.com/devcsrj/828e5be44a54f5217cbd/8f63cd1be9ff89f5c5959316258d75d7805da49f)
`Tests run: 25, Failures: 0, Errors: 0, Skipped: 0`

Hi @nacx ! With the live tests finally passing, you can review the changes 
again. With the latest impl, I chose to delegate *provisioning methods* in  a 
[ProvisioningManager](https://github.com/devcsrj/jclouds-labs/commit/a6ff7dd4f465d94a8063a3ac7a743c1a169406e5#diff-c67d5d74a6059f235c249f2e9a00a176R42),
 which holds a single-threaded executor per *logical group*, in this case, a 
datacenter. WDYT?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-104466542

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

(TODO: Live test)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-104175026

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Interesting. IIRC a similar thing happens in Azure. It is resolved there by 
using a 
[ConflictManagementPredicate](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute/src/main/java/org/jclouds/azurecompute/util/ConflictManagementPredicate.java)
 that tries to complete the operation until it fails or times out. You can see 
an example usage 
[here](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute/src/main/java/org/jclouds/azurecompute/compute/AzureComputeServiceAdapter.java#L146-L159).
 Just a tip!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-103813539

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

[Live test 
results.](https://gist.github.com/devcsrj/828e5be44a54f5217cbd/088324b581f7f528144fc43d2c42cb1f529d60fb)
`Tests run: 25, Failures: 5, Errors: 0, Skipped: 11`
Weirdly enough, when doing debug mode (adding `-Dmaven.surefire.debug` and 
breakpoint at 
[testGet](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java#L521))
 the test doesn't stop, as opposed from previous comment.)

As it turns out, it's not possible to concurrently execute methods in a 
datacenter. (i.e.: when using `client.rebootNodesMatching(..)`, other nodes 
would actually fail to receive the request because the API complains that a 
`PROVISIONING_IN_PROCESS`.) Need to refactor 
`ProfitBricksComputeServiceAdapter` to use a single-thread worker per 
datacenter approach.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-103782440

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

(TODO: Live test)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-103404205

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

>The current issue I need to resolve is that the tests require that the 
>hostname be retrieved via API (which isn't possible). Yet to think a 
>workaround.

Oh, I see. In this case, I'd say don't worry and go ahead overwriting the 
hostname check, as we do in 
[cloudsigma](https://github.com/jclouds/jclouds-labs/blob/master/cloudsigma2/src/test/java/org/jclouds/cloudsigma2/compute/CloudSigma2ComputeServiceLiveTest.java#L51-L54).
 Not all providers offer the same feature set (for example not all providers 
have user metadata, etc), so it is OK to relax/remove some checks. The 
important thing is that the node provisioning flow works as expected and that 
all supported template options are also taken into account.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-103013522

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi @nacx ! While this provider can already provision nodes successfully, I 
haven't been able to pass all tests required by the 
`BaseComputeServiceLiveTest`. The network issue is resolved by increasing the 
timeout. The current issue I need to resolve is that the tests 
[require](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java#L232)
 that the `hostname` be retrieved via API (which isn't possible). Yet to think 
a workaround.

Hi @baldwinSPC , @jasminSPC ! I haven't put much time in this PR in a while, 
but I'd appreciate if you could run the live tests on your end as well and post 
a link to the results as gist (or workaround for the issues too!). Here's the 
command I'm using:
$ `mvn clean integration-test -Plive 
-Dtest=org.jclouds.profitbricks.compute.ProfitBricksTemplateBuilderLiveTest 
-Dtest.profitbricks.identity='' 
-Dtest.profitbricks.credential='' 
-Dtest.profitbricks.template='locationId=16c7ffee-70d8-4505-8793-d2b32795e4fb,imageNameMatches=jclouds-ubuntu14.10-template,loginUser=root:asdfghjk'`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-102963510

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

The only thing I'm missing in this PR is a test that extends the 
[BaseTemplateBuilderLiveTest](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseTemplateBuilderLiveTest.java).
 This test, in conjunction with the adapter one are the compute service 
contract. Once both are passing, the provider is ready to go.

> Encountering what seem to be networking issues in the provider; thus, making 
> jclouds unable to connect via ssh. Needs further investigation.

@devcsrj Is this the cause of the current live test failures? In that case, 
would it be safe to say that the current code should be good to go?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-101966283

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

@nacx is this a PR that can be merged yet? @devcsrj would you like me or 
@jasminSPC to do anything on this to close it out? 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-101770282

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Yes, I'm using a custom image. Not exactly an auth issue (because the 
`testCreateAndRunAService` succeeded; was able to install configure jetty, 
etc). 

Upon further investigation, the servers become inaccessible via ssh after some 
`n` servers are created. Currently, the implementation [creates a lan for every 
server](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-1a27cebd5920714ae4c207026edd5255R143).
 I'll try testing afterwards by explicitly setting which lan to connect. Might 
not be ideal though, thoughts?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-93653258

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

TODO: Live test

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-93653297

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> +Storage.Request.connectingBuilder()
> +.storageId(storageId)
> +.serverId(serverId)
> +.build());
> +
> +waitDcUntilAvailable.apply(dataCenterId);
> +logger.trace(">> storage connected.");
> + } catch (Exception ex) {
> +// delete unconnected storage
> +logger.warn(ex, ">> failed to connect storage '%s'. deleting..", 
> storageId);
> +waitDcUntilAvailable.apply(dataCenterId);
> +destroyStorage(storageId, dataCenterId);
> + }
> +  }
> +
> +  waitDcUntilAvailable.apply(dataCenterId);

Hmm. Yeah it's no longer needed. I was too paranoid of the datacenter not being 
ready I guess. Haha

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r28475366

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Test failures seem to be due to auth issues. Are you using the mentioned custom 
image?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-93447771

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +Storage.Request.connectingBuilder()
> +.storageId(storageId)
> +.serverId(serverId)
> +.build());
> +
> +waitDcUntilAvailable.apply(dataCenterId);
> +logger.trace(">> storage connected.");
> + } catch (Exception ex) {
> +// delete unconnected storage
> +logger.warn(ex, ">> failed to connect storage '%s'. deleting..", 
> storageId);
> +waitDcUntilAvailable.apply(dataCenterId);
> +destroyStorage(storageId, dataCenterId);
> + }
> +  }
> +
> +  waitDcUntilAvailable.apply(dataCenterId);

Is this wait needed? Each step in the loop/catch will already perform an active 
wait.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r28425751

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

[Live test 
results](https://gist.github.com/devcsrj/828e5be44a54f5217cbd/60eca3d2eb1fe1d07bfdec1c9c41ec03d0458059).
`Tests run: 25, Failures: 3, Errors: 0, Skipped: 10, at 38:40.672s`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-93241362

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Yo @baldwinSPC . Haven't tried in the non-dev cluster yet, but will, tomorrow. 
:)

Also, will apply more changes based on conversation with Ignasi at IRC.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-92878168

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

@devcsrj Just checking in on the live testing. How's it going? 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-92873560

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

TODO: Live test

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-92707469

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

TODO: Live test

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-92278075

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

Correct, if you wish to automate against PB you would need to leverage a
custom image that precludes the password change requirement; however, I
believe they are moving towards supporting things such as SSH keys,
user-data via cloud-init, and other things that would alleviate this long
term. It certainly puts a wrinkle in any full automation.

At the moment, we're building the tooling that would allow automation to
start to happen -- working in things like Ansible, Libcloud, Fog, along
with base libraries for their current API. Currently, users build custom
images and then automate against the API if doing infrastructure
automation.

On Tue, Mar 31, 2015 at 1:40 PM, Ignasi Barrera 
wrote:

> Option 2 sounds like the way to go. If we do that, then any user that
> wants to automate stuff when deploying to ProfitBricks with jclouds will
> have to use a custom image that should have been created before, right?
> Out of curiosity, how are your users (PB users) automating stuff?
>
> Thanks for joining the discussion and for your help @baldwinSPC
> !
>
> —
> Reply to this email directly or view it on GitHub
> .
>


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-88494446

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Option 2 sounds like the way to go. If we do that, then any user that wants to 
automate stuff when deploying to ProfitBricks with jclouds will have to use a 
custom image that should have been created before, right?
Out of curiosity, how are your users (PB users) automating stuff?

Thanks for joining the discussion and for your help @baldwinSPC!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-88237905

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

@baldwinSPC SCORE? AWESOME. :+1: 

As soon as you get the confirmation, go ahead and do the snapshot route. :)

Creds via email will do fine. :)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-86817489

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

@devcsrj I should also ask how you'd like us to convey the creds to you for 
testing. :) 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-86696734

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

@devcsrj We've been able to score a test account for jclouds at ProfitBricks. 
We're waiting on the confirmation so we can log in. If we can go the snapshots 
route then we'll build a server and snapshot it. If not, then we'll take a 
different approach. 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-86696408

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi @baldwinSPC ,

(2 + 3) :+1:  Sounds about the right step, since for this provider to be 
promoted out of labs, it'll need a testing account. Btw, can't seem to find the 
_new developer program_. Is there some link or was it via email or something?

(3) This was the suggestion from Flavius (PB S.Engr) as well. Although I'm 
thinking: AFAIK jclouds doesn't necessarily have a concept for _snapshots_. I 
initially thought of maybe, categorizing _snapshots_ as an **image** (i.e.: a 
`Provisionable`, a resource where a node can be created). This way, the 
provider will search for both snapshots and image when creating nodes. I'll ask 
for the commit-ers' opinion after they release the 1.9.0.

In any case, feel free to continue where I left off. Kinda busy on my end, so I 
haven't put much time here yet. :) With that said, thanks for the suggestions!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-85259520

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

Hey @devcsrj 

So, a couple ways we could handle this:

1. We could do the image and push it to our Virtual Data Center, then we 
perform the live tests against our account. 
2. We could setup a dedicated jclouds account using the new Developer Program 
at PB. 
3. You could spin up a VM using their DCD tool, set the password, then create a 
snapshot and use that snapshot as the base image for any build requests. This 
would remove the password change requirement. 

I think long term (2) would be ideal. Short term (3) might be pretty do-able 
depending on your time. Let me know if you'd like us to do (1) and then just 
pass you the creds or have us walk through the steps for the validation. 

-matt

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-85134588

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi @baldwinSPC ! 

I'm still working on passing the live tests for the compute service adapter. 
Right now, the issue is that the default images from ProfitBricks are built 
with password change a requirement upon first login, and so the ssh commands 
executed by tests are actually left hanging. I'm thinking of uploading a custom 
one that we can use for the live test. ([see 
also](https://devops.profitbricks.com/community/an-image-without-password-change/))

With that said, if you could get the live test pass with a viable approach, 
that'll be much much welcome. :)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-78405860

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Matt Baldwin]

@devcsrj @nacx Hey guys, is there any assistance we can provide on this PR or 
is it ready to go? Thanks.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-78288352

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

Yeah, live tests take their time :)

So that password change is an issue, as it is interactive and we can't 
workaround that easily (but if you find a way, it would be very welcome!). The 
same happened with CloudSigma, and we ended up using a template that supported 
cloud-init and did not request the password change.

You can pass the "jclouds.template" property to set the template that the live 
tests will use, and you should configure the property with a template that 
works in the provider's metadata default properties (look at CloudSigma or 
DigitalOcean).

Let's see if there is an image that does not require the password change.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-77327875

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi @nacx !
So 11 of the live tests were green; the failed tests were related to the ssh 
flow like:

org.jclouds.ssh.SshException: 
(root:pw[ca4cb85a391831b122e3277b88393d3f]@46.16.73.251:22) 
(root:pw[ca4cb85a391831b122e3277b88393d3f]@46.16.73.251:22) error acquiring 
ExecResponse(command=[rm /tmp/init-configure-jetty]) (out of retries - max 5): 
Read timed out

I tried to ssh to the said nodes, and it turns out that this was due to the 
operating system requesting a password change after logging in; thus, hanging 
for so long waiting for an input. It's a really weird behavior IMO, since I 
thought this was only applicable if I let the API generate it's own password. 
(I emailed PB tech support regards this.)

Assuming this was by design, my next step would be is to connect via SSH right 
away (to complete the prerequisites first) before returning the 
`NodeAndInitialCredentials`. I'm not sure if there are _hooks_ for something 
like this, but WDYT? Suggestions?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-77312703

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

[Live test 
results](https://gist.github.com/devcsrj/828e5be44a54f5217cbd/5c7509e5e20aa2ddb43dd48ca570b3ba9e1e8754)
`Tests run: 25, Failures: 4, Errors: 0, Skipped: 10` (48:46.720s, ugh.)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-77311677

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +  checkNotNull(from, "Null image");
> +
> +  String desc = from.name();
> +  OsFamily osFamily = parseOsFamily(desc);
> +
> +  OperatingSystem os = OperatingSystem.builder()
> +   .description(osFamily.value())
> +   .family(osFamily)
> +   .version(parseVersion(desc))
> +   .is64Bit(is64Bit(desc))
> +   .build();
> +
> +  return new ImageBuilder()
> +   .ids(from.id())
> +   .name(desc)
> +   .location(mapLocation(from.location()))

The location hierarchy makes sense. jclouds, however, already provides the 
PROVIDER location through the `JustProvider` supplier. You can inject it in the 
DataCenterToLocation function and use it. Have a look at how 
[digitalocean](https://github.com/jclouds/jclouds-labs/blob/master/digitalocean/src/main/java/org/jclouds/digitalocean/compute/functions/RegionToLocation.java)
 does this.

Regarding the images that don't exist in all locations:

* The ComputeServiceAdapter must return each existing image, for each location. 
One image for each different location.
* The providerId of the image must be set to the *real* id of the image in the 
provider.
* The "id" of the image is the one jclouds will use to identify it (for example 
when calling the `geImage(id)` in the ComputeService or when using the 
`imageId` template option. If the same image id is available in multiple 
locations, usually providers encode the region in this field. Something like: 
"us-west-1/image-id". If ids are unique it is ok to use the same id for the 
providerId and the id (what the `ids` method in the builder does). Otherwise 
you'll have to make the "id" field able to distinguish between each image in 
each region. You can have a look at how the OpenStack provider uses the 
[RegionAndId](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/RegionAndId.java)
 class.
* Finally, if images are by default available in **all** locations, the 
"location" field should be left null, otherwise it should be set to the right 
location (region) instead of being set to the PROVIDER scoped one. You can do 
this by injecting the locations supplier here as you do in the 
`ServerToNodeMetadata` to find the location where the image is available. This 
assumes that the ComputeService returns one copy of the image for each location 
where it is available, so you can make sure you return there all you need to 
build this properly here.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25766050

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

[Live test results](https://gist.github.com/devcsrj/828e5be44a54f5217cbd). 
`Status=FAILING`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-77119191

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> +  checkNotNull(from, "Null image");
> +
> +  String desc = from.name();
> +  OsFamily osFamily = parseOsFamily(desc);
> +
> +  OperatingSystem os = OperatingSystem.builder()
> +   .description(osFamily.value())
> +   .family(osFamily)
> +   .version(parseVersion(desc))
> +   .is64Bit(is64Bit(desc))
> +   .build();
> +
> +  return new ImageBuilder()
> +   .ids(from.id())
> +   .name(desc)
> +   .location(mapLocation(from.location()))

Hmm. I couldn't tell if it it qualifies to the __available in all regions__ 
statement. Let me describe it instead.

So given is an image called `Ubuntu-14.04-LTS-server-2015-03-01`. This exists 
in 
[Location](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-8dba16965654b3b80ba6cc57f45b8278L19)s:
 `de/fra`, `de/fkb` but with different `id`s. There's also a _similar_ image 
called `Ubuntu-14.04-LTS-server-2015-03-02` in `us/las` and `us/lasdev` also 
different IDs. Does this qualify to the statement? (Does this present an issue?)

Also, I thought of `LocationScope` as a hierarchy. So `Provider` -> `Region` -> 
`Zone`, etc., and so I mapped it like `Location`(=`Provider`) -> 
`DataCenter`(`=Region`). Correct me if I assumed incorrectly

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25748163

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +  logger.trace("<< searching for server with id=%s", id);
> +
> +  Server server = api.serverApi().getServer(id);
> +  logger.trace(">> found server [%s]", server.name());
> +  return server;
> +   }
> +
> +   @Override
> +   public void destroyNode(String id) {
> +  ServerApi serverApi = api.serverApi();
> +  Server server = serverApi.getServer(id);
> +  if (server != null) {
> + String dataCenterId = server.dataCenter().id();
> + for (Storage storage : server.storages()) {
> +waitDcUntilAvailable.apply(dataCenterId);
> +api.storageApi().deleteStorage(storage.id());

try/catch each operation to delete as much as we can if something fails?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25732611

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

This is a pretty good start @devcsrj! Just commented some things I've seen in a 
quick review, and added an important comment about the live test. If you change 
it to extend the BaseComputeServiceLive test you will see what we expect, and 
you'll be able to build the ComputeService keeping that test as a goal.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-77050364

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +import org.jclouds.profitbricks.domain.Image;
> +import org.jclouds.profitbricks.domain.Server;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.collect.ImmutableList;
> +import com.google.common.collect.Iterables;
> +import com.google.inject.Injector;
> +import com.google.inject.Module;
> +import org.jclouds.compute.ComputeService;
> +import org.jclouds.compute.ComputeServiceAdapter.NodeAndInitialCredentials;
> +import org.jclouds.compute.domain.OsFamily;
> +import org.jclouds.compute.domain.Template;
> +import org.jclouds.compute.domain.TemplateBuilder;
> +
> +@Test(groups = "live", singleThreaded = true, testName = 
> "ProfitBricksComputeServiceAdapterLiveTest")
> +public class ProfitBricksComputeServiceAdapterLiveTest extends 
> BaseProfitBricksLiveTest {

You don't have to build a custom test for this. You must extend the 
[BaseComputeServiceLiveTest](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java).
 That is the jclouds contract. Once that test passes, the ComputeService 
implementation is working as expected. You can take a look at the 
[DigitalOceanComputeServiceLiveTest](https://github.com/jclouds/jclouds-labs/blob/master/digitalocean/src/test/java/org/jclouds/digitalocean/compute/DigitalOceanComputeServiceLiveTest.java)
 for an example.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25733460

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> + return NodeMetadata.Status.RUNNING;
> +  case BLOCKED:
> + return NodeMetadata.Status.PENDING;
> +  case CRASHED:
> + return NodeMetadata.Status.ERROR;
> +  default:
> + return NodeMetadata.Status.UNRECOGNIZED;
> +  }
> +   }
> +
> +   static OperatingSystem mapOsType(OsType osType) {
> +  OperatingSystem os = OperatingSystem.builder()
> +   .description(OsFamily.UNRECOGNIZED.value())
> +   .family(OsFamily.UNRECOGNIZED)
> +   .build();
> +  if (os != null)

This is never null at this point.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25733139

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +  checkNotNull(from, "Null image");
> +
> +  String desc = from.name();
> +  OsFamily osFamily = parseOsFamily(desc);
> +
> +  OperatingSystem os = OperatingSystem.builder()
> +   .description(osFamily.value())
> +   .family(osFamily)
> +   .version(parseVersion(desc))
> +   .is64Bit(is64Bit(desc))
> +   .build();
> +
> +  return new ImageBuilder()
> +   .ids(from.id())
> +   .name(desc)
> +   .location(mapLocation(from.location()))

Images are available in all regions by default? (Just asking as the mapLocation 
function always sets the PROVIDER scope.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25732915

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> + .serverId(serverId)
> + .build());
> +
> + waitDcUntilAvailable.apply(dataCenterId);
> + logger.trace(">> storage connected.");
> +  }
> +
> +  Server server = getNode(serverId);
> +
> +  LoginCredentials serverCredentials = LoginCredentials.builder()
> +  .user("root") // always root user
> +  .password(password)
> +  .build();
> +
> +  return new NodeAndInitialCredentials(server, serverId, 
> serverCredentials);
> +   }

This method creates several resources in the provider. If it fails at some 
point, we should take care of rolling back the created stuff (at least we 
should try), in order to avoid leaking resources.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25732565

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> + .size(volume.getSize())
> + .build());
> + waitDcUntilAvailable.apply(dataCenterId);
> + storageIds.add(id);
> + logger.trace(">> provisioning complete for storage. returned 
> id='%s'", id);
> +  }
> +
> +  Double cores = 0d;
> +  for (Processor processor : hardware.getProcessors())
> + cores += processor.getCores();
> +
> +  String storageBootDeviceId = Iterables.getFirst(storageIds, "");
> +  Server.Request.CreatePayload serverRequest = 
> Server.Request.creatingBuilder()
> +  .dataCenterId(dataCenterId)
> +  .name(name)
> +  .bootFromStorageId(storageBootDeviceId)

What happens when the value is an empty string?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25732385

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +
> +@Singleton
> +public class ProfitBricksComputeServiceAdapter implements 
> ComputeServiceAdapter {
> +
> +   @Resource
> +   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
> +   protected Logger logger = Logger.NULL;
> +
> +   private final ProfitBricksApi api;
> +   private final Predicate waitDcUntilAvailable;
> +
> +   @Inject
> +   ProfitBricksComputeServiceAdapter(ProfitBricksApi api) {
> +  this.api = api;
> +  this.waitDcUntilAvailable = Predicates2.retry(new 
> ProvisioningStatusPollingPredicate(
> +  api, ProvisioningStatusAware.DATACENTER, 
> ProvisioningState.AVAILABLE), 10l * 60l, 2l, TimeUnit.SECONDS);

Consider making the predicate injectable. That would facilitate making the 
timeouts configurable so users can customise them. Take a look at 
[Azure](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute/src/main/java/org/jclouds/azurecompute/compute/config/AzureComputeServiceContextModule.java#L95-L101)
 to see an example.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25732194

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +import org.jclouds.profitbricks.compute.function.ImageToImage;
> +import org.jclouds.profitbricks.compute.function.ServerToNodeMetadata;
> +import org.jclouds.profitbricks.compute.function.StorageToVolume;
> +
> +import com.google.common.base.Function;
> +import com.google.inject.TypeLiteral;
> +
> +public class ProfitBricksComputeServiceContextModule extends
> + ComputeServiceAdapterContextModule 
> {
> +
> +   @Override
> +   protected void configure() {
> +  super.configure();
> +
> +  // FIXME Not working
> +  install(new LocationsFromComputeServiceAdapterModule Image, DataCenter>() {

That's because it is qualified when bound in the module that configures it:
https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/location/config/LocationModule.java#L102-L106

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25674714

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +import org.jclouds.profitbricks.compute.function.ImageToImage;
> +import org.jclouds.profitbricks.compute.function.ServerToNodeMetadata;
> +import org.jclouds.profitbricks.compute.function.StorageToVolume;
> +
> +import com.google.common.base.Function;
> +import com.google.inject.TypeLiteral;
> +
> +public class ProfitBricksComputeServiceContextModule extends
> + ComputeServiceAdapterContextModule 
> {
> +
> +   @Override
> +   protected void configure() {
> +  super.configure();
> +
> +  // FIXME Not working
> +  install(new LocationsFromComputeServiceAdapterModule Image, DataCenter>() {

I think you have to qualify the supplier as `@Memoized` in the 
`ServerToNodeMetadata`. Something like:
```java
@Inject
 ServerToNodeMetadata(Function fnVolume,
  @Memoized Supplier> locationsSupply) {
```


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25674540

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Ignasi Barrera]

> +  .serverId(serverId)
> +  .build());
> +
> +  waitDcUntilAvailable.apply(dataCenterId);
> +  logger.trace("<< storage connected.");
> +  }
> +
> +  Server server = getNode(serverId);
> +
> +  // TODO Consult jclouds committers. Server credentials in profitbricks 
> are sent 
> +  // via email used in credentials.identity. We can set a password 
> during 
> +  // storage provisioning, maybe generate random characters?
> +  LoginCredentials serverCredentials = LoginCredentials.builder()
> +   .user("root")
> +   .password("")
> +   .build();

jclouds needs the node credentials to be able to connect to the node when using 
the "runScript" methods. If credentials are configured per-node (instead of 
having default credentials per image), we should return them here.

If the password can be set during the node provisioning, that's perfect. If the 
user has set a password using the `templateOptions.overrideLoginPassword`, 
let's use that one, otherwise auto-generate a random one. The same applies to 
the user.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25674213

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> +import org.jclouds.profitbricks.compute.function.ImageToImage;
> +import org.jclouds.profitbricks.compute.function.ServerToNodeMetadata;
> +import org.jclouds.profitbricks.compute.function.StorageToVolume;
> +
> +import com.google.common.base.Function;
> +import com.google.inject.TypeLiteral;
> +
> +public class ProfitBricksComputeServiceContextModule extends
> + ComputeServiceAdapterContextModule 
> {
> +
> +   @Override
> +   protected void configure() {
> +  super.configure();
> +
> +  // FIXME Not working
> +  install(new LocationsFromComputeServiceAdapterModule Image, DataCenter>() {

For some reason, this module doesn't get installed. Guice still complains that 
there are no bound implementation for `Supplier>` when used in the 
[ServerToNodeMetadata](https://github.com/jclouds/jclouds-labs/pull/145/files#diff-56ae535b8c57334bd467e33f42c740b9R63)
 constructor.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25661831

Re: [jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

> +  .serverId(serverId)
> +  .build());
> +
> +  waitDcUntilAvailable.apply(dataCenterId);
> +  logger.trace("<< storage connected.");
> +  }
> +
> +  Server server = getNode(serverId);
> +
> +  // TODO Consult jclouds committers. Server credentials in profitbricks 
> are sent 
> +  // via email used in credentials.identity. We can set a password 
> during 
> +  // storage provisioning, maybe generate random characters?
> +  LoginCredentials serverCredentials = LoginCredentials.builder()
> +   .user("root")
> +   .password("")
> +   .build();

(on comment //) Recommendation?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145/files#r25661894

[jclouds-labs] JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter (#145)

[Reijhanniel Jearl Campos]

Hi,

This implements `ProfitBricksComputeServiceAdapter`. The PR is expected to 
fail, since I have yet to resolve (which I need an eye from someone):

Aside from the mentioned issues, the `Function` classes are ready to be 
reviewed.

/cc @nacx 
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds-labs/pull/145

-- Commit Summary --

  * JCLOUDS-702: JClouds ProfitBricks provider - ComputeServiceAdapter

-- File Changes --

M 
profitbricks/src/main/java/org/jclouds/profitbricks/ProfitBricksApiMetadata.java
 (4)
M 
profitbricks/src/main/java/org/jclouds/profitbricks/binder/datacenter/CreateDataCenterRequestBinder.java
 (2)
A 
profitbricks/src/main/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapter.java
 (267)
A 
profitbricks/src/main/java/org/jclouds/profitbricks/compute/function/DataCenterToLocation.java
 (54)
A 
profitbricks/src/main/java/org/jclouds/profitbricks/compute/function/ImageToImage.java
 (85)
A 
profitbricks/src/main/java/org/jclouds/profitbricks/compute/function/ServerToNodeMetadata.java
 (172)
A 
profitbricks/src/main/java/org/jclouds/profitbricks/compute/function/StorageToVolume.java
 (47)
A 
profitbricks/src/main/java/org/jclouds/profitbricks/config/ProfitBricksComputeServiceContextModule.java
 (69)
M profitbricks/src/main/java/org/jclouds/profitbricks/domain/Location.java 
(19)
M profitbricks/src/main/java/org/jclouds/profitbricks/domain/Server.java 
(27)
M 
profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/server/BaseServerResponseHandler.java
 (9)
M 
profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/server/ServerInfoResponseHandler.java
 (1)
A 
profitbricks/src/test/java/org/jclouds/profitbricks/compute/ProfitBricksComputeServiceAdapterLiveTest.java
 (147)
A 
profitbricks/src/test/java/org/jclouds/profitbricks/compute/function/DataCenterToLocationTest.java
 (70)
A 
profitbricks/src/test/java/org/jclouds/profitbricks/compute/function/ImageToImageTest.java
 (168)
A 
profitbricks/src/test/java/org/jclouds/profitbricks/compute/function/ServerToNodeMetadataTest.java
 (171)
A 
profitbricks/src/test/java/org/jclouds/profitbricks/compute/function/StorageToVolumeTest.java
 (63)
M 
profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/image/ImageListResponseHandlerTest.java
 (6)
M 
profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/server/ServerInfoResponseHandlerTest.java
 (5)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/145.patch
https://github.com/jclouds/jclouds-labs/pull/145.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/145