You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2016/05/20 22:52:25 UTC

[jclouds/jclouds] Allow to link contexts and views together (#960)

Allow to link contexts and views together, so one provider can inject and call another. This adds the `linkedContexts`and `linkedViews` methods to the `ContextBuilder`.

An example usage to link an existing compute context with a blob store one would be something like:

```java
BlobStoreContext blobstore = ContextBuilder.newBuilder("azureblob")
   .credentials("identity", "credential")
   ...
   .buildView(BlobStoreContext.class);

ComputeServiceContext compute = ContextBuilder.newBuilder("azurecompute-arm")
   .credentials("identity", "credential")
   .linkedViews(blobstore)
   ...
   .buildView(BlobStoreContext.class);
```

Providing a link to a context or view during context creation, makes the linked entities injectable by "provider id". An example code in the compute provider could be:

```java
public class DependsOnTheExternalBlob {
   private final BlobStoreContext blobstore;

   @Inject DependsOnTheExternalBlob(@Named("azureblob") View blobstoreView) {
      checkArgument(blobstoreView instanceof BlobStoreContext);
      blobstore = (BlobStoreContext) blobstoreView;
   }
}
```

Note a couple things:

* Linked contexts and views are bound using the provider id. This allows to link more than one provider and inject the right one where needed.
* Injection has to use the base `View` and `Context` types. The jclouds-core project does not have teh BlobStoreContext or ComputeService views; they're defined in downstream projects, so we can't create a binding for that type when adding the linked views to the injector.

And finally, specifics for the Azure ARM ImageExtension:

* Since the extension is optional, one could consider to mark the injection as optional, to avoid failing to create the context if users don't provide the linked context.
* This PR also provides a commit to allow providers to override the binding for the image extension. This way the Azure provider will be able to return an absent extension if there is no linked azure blob context.

@jtjk Could you try building this branch and see if it helps you build the Azure ARM ImageExtension?
@demobox Could you have a look and give your opinion about this approach to allowing to call external contexts?
@andreaturli @zack-shoylev This will potentially help calling Neutron from the nova compute service (although first we need to promote Neutron to this repo). WDYT about this?
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/960

-- Commit Summary --

  * Allow to link contexts and views together
  * Allow to override the Image and Security extension bindings

-- File Changes --

    M compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java (14)
    M core/src/main/java/org/jclouds/ContextBuilder.java (24)
    A core/src/main/java/org/jclouds/config/BindLinkedContextsAndViews.java (90)
    M core/src/test/java/org/jclouds/ContextBuilderTest.java (59)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/960.patch
https://github.com/jclouds/jclouds/pull/960.diff

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/960

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
>It is not that many azure api calls... maybe 2 or 3.

Then I'd propose to create an API in the ARM provider that implements those calls. It is overkill to create contexts just for that, and it simplifies a lot the providers and the user experience.

>it looks extremely useful to me, particularly for the Neutron use-case. The fix looks also relatively simple so why not ;)

@demobox good to merge?

---
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/pull/960#issuecomment-221208748

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
I think that error appears when the slave dies? Anyway, build is passing now, so... merged to master!
Thanks everyone for the 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/pull/960#issuecomment-221683263

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

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

---
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/pull/960#event-672270013

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
> @demobox good to merge

Looks good to me - thanks for making the changes, @nacx!

@andreaturli @zack-shoylev Any chance of testing whether this change is enough to allow the Neutron use case to succeed? Or are we sufficiently confident of 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/pull/960#issuecomment-221410683

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Janne Koskinen <no...@github.com>.
You have understood correctly. 

To get list of custom images: All normal VMs have storage accounts and we need to use StorageAccountAPI.getKeys to each storage account and then we need to use blob store API with those keys to list all custom images. => we need N contexts. 

It is not that many azure api calls... maybe 2 or 3.

---
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/pull/960#issuecomment-221197127

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Janne Koskinen <no...@github.com>.
Hi,

Thanks for all the help @nacx. Now when planning to try this, I realize that there is one problem with this approach and Azure ImageExtension. The custom image is created to blob storage of the copied VM and we need to use VM storage keys as credentials. We can only get those credentials by calling VirtualMachineAPI.getKeys inside azurecompute-arm. 

Is it possible to link unauthenticated BlobStoreContext?

---
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/pull/960#issuecomment-220899747

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
I'm a bit confused what do you need the blob store api exactly:

* To create the image for a given VM.
* To list custom images.
* For both?

---
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/pull/960#issuecomment-221122836

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
In that case:

* In your first point, we need the blob store api to copy the image to the new storage, right? We'd need the blob store dependency outside the image extension. And to do that, we need the keys from the original vm. Is this right?
* To list all vms, we need the blob store api too. Do we need to use the credentials of *each* vm (thus creating N blobstore contexts)? And then, how do we know which contexts to create? Do we have to list all blobs or sonething like that?

If we end up needing to create N contexts, which is expensive, or depending always on the blobstore api in the adapter, I'd perhaps consider adding a blob store api in the arm provider directly.

* Which different azure api calls are needed to cover the cases above? If they are not many, I think the best approach is to add just those methods to a new api in arm, and let those methods get thw VM credentials as parameters. A particular endpoint can be configured too.

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/960#issuecomment-221194553

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
Closed #960.

---
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/pull/960#event-671168376

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
> merged to master!

Yay - thanks @nacx!

---
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/pull/960#issuecomment-221979549

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Janne Koskinen <no...@github.com>.
- If we want to have own storage accounts for virtual machines created from custom images, we need blob store api to create new virtual machines.
- We don't need blob store api for creating the custom image. 
- We need blob store api to list custom images



---
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/pull/960#issuecomment-221170749

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
> Could you have a look and give your opinion about this approach to allowing to call external contexts?

Will try to take a look over the weekend. Thanks for all the work in putting this together, @nacx!

---
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/pull/960#issuecomment-220759317

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
BTW, with he latest changes, the way to use it is pretty much the same, but we allow users to directly provide the view/context or a supplier to lazily create it. Following the example in the PR description, the way to use it now would be like:

```java
BlobStoreContext blobstore = ContextBuilder.newBuilder("azureblob")
   .credentials("identity", "credential")
   ...
   .buildView(BlobStoreContext.class);

ComputeServiceContext compute = ContextBuilder.newBuilder("azurecompute-arm")
   .credentials("identity", "credential")
   .modules(ImmutableSet.of(ContextLinking.linkToView(blobstore)))
   ...
   .buildView(ComputeServiceContext.class);
```

And to get it injected (note that regardless of how the user configures the module we always bind a Supplier):

```java
public class DependsOnTheExternalBlob {
   private final Supplier<View> blobstore;

   @Inject DependsOnTheExternalBlob(@Named("azureblob") Supplier<View> blobstore) {
      blobstore = blobstore;
   }
}
```

---
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/pull/960#issuecomment-221209582

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrea Turli <no...@github.com>.
Thanks @nacx it looks extremely useful to me, particularly for the Neutron use-case. The fix looks also relatively simple so why not ;)

---
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/pull/960#issuecomment-221206575

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Rita Zhang <no...@github.com>.
@nacx This is to list custom images since they are stored in a storage account as blobs.

---
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/pull/960#issuecomment-221126930

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * Utility methods to allow {@link Context} and {@link View} linking between
> + * contexts.
> + * <p>
> + * By using this module users can configure a context to be able to inject other
> + * contexts or views by their provider id.
> + */
> +public class ContextLinking {
> +
> +   static final TypeLiteral<Supplier<Context>> CONTEXT_SUPPLIER = new TypeLiteral<Supplier<Context>>() {
> +   };
> +
> +   static final TypeLiteral<Supplier<View>> VIEW_SUPPLIER = new TypeLiteral<Supplier<View>>() {
> +   };
> +
> +   public static Module linkToView(final String id, final Supplier<View> view) {

[minor] Perhaps call this `linkView` or `linkedView` rather than `linkToView`? If I read it, the idea that I'm "linking this view" and "linking that context" sounds a bit closer to what's happening than "I'm linking _to_ that view"?

---
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/pull/960/files/d7aa6e5aff7b2858c581952ca90c8021d07cac93#r64480233

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
Addressed the comments and squashed the commits.

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/960#issuecomment-221416831

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
Reopened #960.

---
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/pull/960#event-671168398

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Ignasi Barrera <no...@github.com>.
There is no way of creating an unauthenticated one, because credentials are injected in many places. I think a good solution would be to internally bind a `Supplier<Context>` and `Supplier<View>` so users can provide an already existing context, or a supplier that will lazily create it when needed. This makes more sense also in the generic case.

In the case of Azure, it will be a bit tricky, as the supplier will also need compute stuff to get the keys from the VM (cyclyc dependency), but I'm pretty sure that will be feasible with a couple changes. I'll update the PR and try to provide some proposed examples of how it would work.

---
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/pull/960#issuecomment-220909659

Re: [jclouds/jclouds] Allow to link contexts and views together (#960)

Posted by Andrew Phillips <no...@github.com>.
> 1 failing check

Jenkins error?

```
[ERROR] Internal error: java.lang.IllegalStateException: Invalid object ID 62 iota=106: Object was recently deallocated
[ERROR] #62 (ref.0) : object=null type=com.sun.proxy.$Proxy86 interfaces=[hudson.maven.MavenBuildProxy2]
[ERROR] Created at Tue May 24 18:11:35 EDT 2016
...
```

Kicking this off again...

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/960#issuecomment-221452006